diff mbox

[ifcvt,1/3] Factor out cost calculations from noce cases

Message ID 1443193479-10830-2-git-send-email-james.greenhalgh@arm.com
State New
Headers show

Commit Message

James Greenhalgh Sept. 25, 2015, 3:04 p.m. UTC
Hi,

In this patch we try to pull out the cost calculations used by the
no-conditional-execution if-convert functions. We want to replicate the
logic of the current cost decisions, but to phrase it in a way which
can be pulled out as common. To preserve the current behaviour as best
as we can, this means asking the common question, "is a magic_number
less than or equal to branch_cost". Clearly this is not the question
we want to be asking longer term, but this preserves existing target
behaviour.

This is imperfect for a few reasons.

First, some of the more ambitious noce if-convert functions have a
(slightly) more complicated cost-model. This means that we have to jump
through hoops to present the cost calculation in the common form. These
hoops are not very big, but it does make the logic seem a bit... weird.

Second, because our long term goal is to hand the cost calculation off
to the target and make it better reflect a meaningful question, we must
first build the candidate ifcvt sequence for comparison. This will cause
a slight compile time regression as we now generate more sequences before
bailing out (each of which needs a cost calculation).

On the other hand, it should be clear from this point what we have to do
to lift this out to a target hook which can do a smart job, and I think
this fits with the overall direction we intend to take.

Bootstrapped and checked on x86_64-none-linux-gnu, aarch64-none-linux-gnu
and arm-none-linux-gnueabihf without issue. Comparison of Spec2000/Spec2006
code generation for these three targets showed no changes.

OK?

Thanks,
James

---
2015-09-26  James Greenhalgh  <james.greenhalgh@arm.com>

	* ifcvt.c (noce_if_info): Add a magic_number field :-(.
	(noce_is_profitable_p): New.
	(noce_try_store_flag_constants): Move cost calculation
	to after sequence generation, factor it out to noce_is_profitable_p.
	(noce_try_addcc): Likewise.
	(noce_try_store_flag_mask): Likewise.
	(noce_try_cmove): Likewise.
	(noce_try_cmove_arith): Likewise.
	(noce_try_sign_mask): Add comment regarding cost calculations.
diff mbox

Patch

diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index 157a716..e89d567 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -820,6 +820,14 @@  struct noce_if_info
 
   /* Estimated cost of the particular branch instruction.  */
   unsigned int branch_cost;
+
+  /* For if-convert transformations, the legacy way to decide whether
+     the transformation should be applied is a comparison of a magic
+     number against BRANCH_COST.  Ultimately, this should go away, but
+     to avoid regressing targets this field encodes that number so the
+     profitability analysis can remain unchanged.  */
+  unsigned int magic_number;
+
 };
 
 static rtx noce_emit_store_flag (struct noce_if_info *, rtx, int, int);
@@ -836,6 +844,19 @@  static rtx noce_get_alt_condition (struct noce_if_info *, rtx, rtx_insn **);
 static int noce_try_minmax (struct noce_if_info *);
 static int noce_try_abs (struct noce_if_info *);
 static int noce_try_sign_mask (struct noce_if_info *);
+static bool noce_is_profitable_p (rtx_insn *, struct noce_if_info *);
+
+/* Given SEQ, which is a sequence we might want to generate after
+   if-conversion, and a basic-block structure in IF_INFO which represents
+   the code generation before if-conversion, return TRUE if this would
+   be a profitable transformation.  */
+
+static bool
+noce_is_profitable_p (rtx_insn *seq ATTRIBUTE_UNUSED,
+		      struct noce_if_info *if_info)
+{
+  return (if_info->branch_cost >= if_info->magic_number);
+}
 
 /* Helper function for noce_try_store_flag*.  */
 
@@ -1192,8 +1213,13 @@  noce_try_store_flag_constants (struct noce_if_info *if_info)
   HOST_WIDE_INT itrue, ifalse, diff, tmp;
   int normalize;
   bool can_reverse;
+  bool no_cost_model = false;
   machine_mode mode = GET_MODE (if_info->x);;
   rtx common = NULL_RTX;
+  /* ??? There are paths through this function from which no cost function
+     is checked before conversion.  Maintain that behaviour by setting
+     the magic number used by noce_is_profitable_p to zero.  */
+  if_info->magic_number = 0;
 
   rtx a = if_info->a;
   rtx b = if_info->b;
@@ -1204,9 +1230,9 @@  noce_try_store_flag_constants (struct noce_if_info *if_info)
       && CONST_INT_P (XEXP (a, 1))
       && CONST_INT_P (XEXP (b, 1))
       && rtx_equal_p (XEXP (a, 0), XEXP (b, 0))
-      && noce_operand_ok (XEXP (a, 0))
-      && if_info->branch_cost >= 2)
+      && noce_operand_ok (XEXP (a, 0)))
     {
+      if_info->magic_number = 2;
       common = XEXP (a, 0);
       a = XEXP (a, 1);
       b = XEXP (b, 1);
@@ -1278,23 +1304,33 @@  noce_try_store_flag_constants (struct noce_if_info *if_info)
 	  else
 	    gcc_unreachable ();
 	}
-      else if (ifalse == 0 && exact_log2 (itrue) >= 0
-	       && (STORE_FLAG_VALUE == 1
-		   || if_info->branch_cost >= 2))
-	normalize = 1;
-      else if (itrue == 0 && exact_log2 (ifalse) >= 0 && can_reverse
-	       && (STORE_FLAG_VALUE == 1 || if_info->branch_cost >= 2))
+      else if (ifalse == 0 && exact_log2 (itrue) >= 0)
 	{
+	  if_info->magic_number = 2;
+	  if (STORE_FLAG_VALUE == 1)
+	    no_cost_model = true;
+	  normalize = 1;
+	}
+      else if (itrue == 0 && exact_log2 (ifalse) >= 0 && can_reverse)
+	{
+	  if_info->magic_number = 2;
+	  if (STORE_FLAG_VALUE == 1)
+	    no_cost_model = true;
 	  normalize = 1;
 	  reversep = true;
 	}
-      else if (itrue == -1
-	       && (STORE_FLAG_VALUE == -1
-		   || if_info->branch_cost >= 2))
-	normalize = -1;
-      else if (ifalse == -1 && can_reverse
-	       && (STORE_FLAG_VALUE == -1 || if_info->branch_cost >= 2))
+      else if (itrue == -1)
 	{
+	  if_info->magic_number = 2;
+	  if (STORE_FLAG_VALUE == -1)
+	    no_cost_model = true;
+	  normalize = -1;
+	}
+      else if (ifalse == -1 && can_reverse)
+	{
+	  if_info->magic_number = 2;
+	  if (STORE_FLAG_VALUE == -1)
+	    no_cost_model = true;
 	  normalize = -1;
 	  reversep = true;
 	}
@@ -1385,6 +1421,10 @@  noce_try_store_flag_constants (struct noce_if_info *if_info)
       if (!seq)
 	return FALSE;
 
+      /* Check if this is actually beneficial.  */
+      if (!no_cost_model && !noce_is_profitable_p (seq, if_info))
+	return FALSE;
+
       emit_insn_before_setloc (seq, if_info->jump,
 			       INSN_LOCATION (if_info->insn_a));
       return TRUE;
@@ -1446,8 +1486,7 @@  noce_try_addcc (struct noce_if_info *if_info)
 
       /* If that fails, construct conditional increment or decrement using
 	 setcc.  */
-      if (if_info->branch_cost >= 2
-	  && (XEXP (if_info->a, 1) == const1_rtx
+      if ((XEXP (if_info->a, 1) == const1_rtx
 	      || XEXP (if_info->a, 1) == constm1_rtx))
         {
 	  start_sequence ();
@@ -1477,6 +1516,11 @@  noce_try_addcc (struct noce_if_info *if_info)
 	      if (!seq)
 		return FALSE;
 
+	      /* Check if this is actually beneficial.  */
+	      if_info->magic_number = 2;
+	      if (!noce_is_profitable_p (seq, if_info))
+		return FALSE;
+
 	      emit_insn_before_setloc (seq, if_info->jump,
 				       INSN_LOCATION (if_info->insn_a));
 	      return TRUE;
@@ -1501,15 +1545,14 @@  noce_try_store_flag_mask (struct noce_if_info *if_info)
     return FALSE;
 
   reversep = 0;
-  if ((if_info->branch_cost >= 2
-       || STORE_FLAG_VALUE == -1)
-      && ((if_info->a == const0_rtx
+
+  if ((if_info->a == const0_rtx
 	   && rtx_equal_p (if_info->b, if_info->x))
 	  || ((reversep = (reversed_comparison_code (if_info->cond,
 						     if_info->jump)
 			   != UNKNOWN))
 	      && if_info->b == const0_rtx
-	      && rtx_equal_p (if_info->a, if_info->x))))
+	      && rtx_equal_p (if_info->a, if_info->x)))
     {
       start_sequence ();
       target = noce_emit_store_flag (if_info,
@@ -1523,7 +1566,7 @@  noce_try_store_flag_mask (struct noce_if_info *if_info)
 
       if (target)
 	{
-	  int old_cost, new_cost, insn_cost;
+	  int new_cost, old_cost;
 	  int speed_p;
 
 	  if (target != if_info->x)
@@ -1533,12 +1576,26 @@  noce_try_store_flag_mask (struct noce_if_info *if_info)
 	  if (!seq)
 	    return FALSE;
 
+	  /* The previous costing code here calculated everything in the
+	     rtx_cost base, and compared it against
+	     COSTS_N_INSNS (if_info->branch_cost).  As we don't want to
+	     multiply branch cost, instead divide through by
+	     COSTS_N_INSNS (1) to get our magic_number.  We also need to
+	     take account of a possible magic_number of 2 which should
+	     be applied if STORE_FLAG_VALUE != -1.  */
 	  speed_p = optimize_bb_for_speed_p (BLOCK_FOR_INSN (if_info->insn_a));
-	  insn_cost = insn_rtx_cost (PATTERN (if_info->insn_a), speed_p);
-	  old_cost = COSTS_N_INSNS (if_info->branch_cost) + insn_cost;
+	  old_cost = insn_rtx_cost (PATTERN (if_info->insn_a), speed_p);
 	  new_cost = seq_cost (seq, speed_p);
+	  if_info->magic_number = (new_cost - old_cost);
+	  if_info->magic_number
+	    = (if_info->magic_number / COSTS_N_INSNS (1))
+	      + ((if_info->magic_number % COSTS_N_INSNS (1)) ? 1 : 0);
 
-	  if (new_cost > old_cost)
+	  if_info->magic_number
+	    = MAX (((STORE_FLAG_VALUE != -1) ? 2 : 0),
+		   (if_info->magic_number));
+
+	  if (!noce_is_profitable_p (seq, if_info))
 	    return FALSE;
 
 	  emit_insn_before_setloc (seq, if_info->jump,
@@ -1703,9 +1760,7 @@  noce_try_cmove (struct noce_if_info *if_info)
 	 we don't know about, so give them a chance before trying this
 	 approach.  */
       else if (!targetm.have_conditional_execution ()
-		&& CONST_INT_P (if_info->a) && CONST_INT_P (if_info->b)
-		&& ((if_info->branch_cost >= 2 && STORE_FLAG_VALUE == -1)
-		    || if_info->branch_cost >= 3))
+		&& CONST_INT_P (if_info->a) && CONST_INT_P (if_info->b))
 	{
 	  machine_mode mode = GET_MODE (if_info->x);
 	  HOST_WIDE_INT ifalse = INTVAL (if_info->a);
@@ -1744,6 +1799,12 @@  noce_try_cmove (struct noce_if_info *if_info)
 	      if (!seq)
 		return FALSE;
 
+	      /* Check if this is actually beneficial.  */
+	      if_info->magic_number = STORE_FLAG_VALUE == -1
+				      ? 2 : 3;
+	      if (!noce_is_profitable_p (seq, if_info))
+		return FALSE;
+
 	      emit_insn_before_setloc (seq, if_info->jump,
 				   INSN_LOCATION (if_info->insn_a));
 	      return TRUE;
@@ -1930,7 +1991,9 @@  noce_try_cmove_arith (struct noce_if_info *if_info)
      conditional on their addresses followed by a load.  Don't do this
      early because it'll screw alias analysis.  Note that we've
      already checked for no side effects.  */
-  /* ??? FIXME: Magic number 5.  */
+  /* ??? FIXME: Magic number 5.  We cost this here rather than through
+     noce_is_profitable_p as the fallback cases below can produce viable
+     transformations in the case where (if_info->branch_cost < 5).  */
   if (cse_not_expected
       && MEM_P (a) && MEM_P (b)
       && MEM_ADDR_SPACE (a) == MEM_ADDR_SPACE (b)
@@ -1973,10 +2036,12 @@  noce_try_cmove_arith (struct noce_if_info *if_info)
   else
     else_cost = 0;
 
-  /* We're going to execute one of the basic blocks anyway, so
-     bail out if the most expensive of the two blocks is unacceptable.  */
-  if (MAX (then_cost, else_cost) > COSTS_N_INSNS (if_info->branch_cost))
-    return FALSE;
+  /* We want the most expensive of the above, divided through by
+     COSTS_N_INSNS (1).  */
+  if_info->magic_number = MAX (then_cost, else_cost);
+  if_info->magic_number
+    = (if_info->magic_number / COSTS_N_INSNS (1))
+      + ((if_info->magic_number % COSTS_N_INSNS (1)) ? 1 : 0);
 
   /* Possibly rearrange operands to make things come out more natural.  */
   if (reversed_comparison_code (if_info->cond, if_info->jump) != UNKNOWN)
@@ -2115,6 +2180,9 @@  noce_try_cmove_arith (struct noce_if_info *if_info)
   if (!ifcvt_seq)
     return FALSE;
 
+  if (!noce_is_profitable_p (ifcvt_seq, if_info))
+    return FALSE;
+
   emit_insn_before_setloc (ifcvt_seq, if_info->jump,
 			   INSN_LOCATION (if_info->insn_a));
   return TRUE;
@@ -2571,7 +2639,12 @@  noce_try_sign_mask (struct noce_if_info *if_info)
      non-zero (T) value and if INSN_B was taken from TEST_BB, or there was no
      INSN_B which can happen for e.g. conditional stores to memory.  For the
      cost computation use the block TEST_BB where the evaluation will end up
-     after the transformation.  */
+     after the transformation.
+     ??? The underlying calculation is a natural fit for the long term
+     direction of noce_is_profitable_p, but there is no way to transform
+     this cost calculation in to a comparison against branch_cost.  When
+     noce_is_profitable_p becomes a proper cost calulcation, this logic
+     should be cleaned up.  */
   t_unconditional =
     (t == if_info->b
      && (if_info->insn_b == NULL_RTX