diff mbox

[RFC:,5/6,v2] Improve the cost model for multiple-sets

Message ID 1466524231-17412-5-git-send-email-james.greenhalgh@arm.com
State New
Headers show

Commit Message

James Greenhalgh June 21, 2016, 3:50 p.m. UTC 
Hi,

This patch is rewrites the cost model for bb_ok_for_noce_multiple_sets
to use the max_seq_cost heuristic added in earlier patch revisions.

As with the previous patch, I've used the new parameters to ensure that
the testsuite is still testing the functionality rather than relying on
the target setting the costs appropriately.

Thanks,
James

---
gcc/

2016-06-21  James Greenhalgh  <james.greenhalgh@arm.com>

	* ifcvt.c (noce_convert_multiple sets): Move cost model to here,
	check the sequence cost after constructing the converted sequence.
	(bb_of_for_noce_convert_multiple_sets): Move cost model.

gcc/testsuite/

2016-06-21  James Greenhalgh  <james.greenhalgh@arm.com>

	* gcc.dg/ifcvt-4.c: Use parameter to guide if-conversion heuristics.
	* gcc.dg/ifcvt-5.c: Use parameter to guide if-conversion heuristics.
diff mbox

Patch

diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index 78906d3..8f892b0 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -3191,6 +3191,7 @@  noce_convert_multiple_sets (struct noce_if_info *if_info)
   rtx_insn *jump = if_info->jump;
   rtx_insn *cond_earliest;
   rtx_insn *insn;
+  bool speed_p = optimize_bb_for_speed_p (if_info->test_bb);
 
   start_sequence ();
 
@@ -3273,9 +3274,17 @@  noce_convert_multiple_sets (struct noce_if_info *if_info)
   for (int i = 0; i < count; i++)
     noce_emit_move_insn (targets[i], temporaries[i]);
 
-  /* Actually emit the sequence.  */
+  /* Actually emit the sequence if it isn't too expensive.  */
   rtx_insn *seq = get_insns ();
 
+  /*  Check the cost model to ensure this is profitable.  */
+  if (seq_cost (seq, speed_p)
+      > if_info->max_seq_cost)
+    {
+      end_sequence ();
+      return FALSE;
+    }
+
   for (insn = seq; insn; insn = NEXT_INSN (insn))
     set_used_flags (insn);
 
@@ -3325,23 +3334,16 @@  noce_convert_multiple_sets (struct noce_if_info *if_info)
 
 /* Return true iff basic block TEST_BB is comprised of only
    (SET (REG) (REG)) insns suitable for conversion to a series
-   of conditional moves.  FORNOW: Use II to find the expected cost of
-   the branch into/over TEST_BB.
-
-   TODO: This creates an implicit "magic number" for if conversion.
-   II->max_seq_cost now guides the maximum number of set instructions in
-   a basic block which is considered profitable to completely
-   if-convert.  */
+   of conditional moves.  Also check that we have more than one set
+   (other routines can handle a single set better than we would), and
+   fewer than PARAM_MAX_RTL_IF_CONVERSION_INSNS sets.  */
 
 static bool
-bb_ok_for_noce_convert_multiple_sets (basic_block test_bb,
-				      struct noce_if_info *ii)
+bb_ok_for_noce_convert_multiple_sets (basic_block test_bb)
 {
   rtx_insn *insn;
   unsigned count = 0;
   unsigned param = PARAM_VALUE (PARAM_MAX_RTL_IF_CONVERSION_INSNS);
-  /* TODO:  Revisit this cost model.  */
-  unsigned limit = MIN (ii->max_seq_cost / COSTS_N_INSNS (1), param);
 
   FOR_BB_INSNS (test_bb, insn)
     {
@@ -3377,14 +3379,15 @@  bb_ok_for_noce_convert_multiple_sets (basic_block test_bb,
       if (!can_conditionally_move_p (GET_MODE (dest)))
 	return false;
 
-      /* FORNOW: Our cost model is a count of the number of instructions we
-	 would if-convert.  This is suboptimal, and should be improved as part
-	 of a wider rework of branch_cost.  */
-      if (++count > limit)
-	return false;
+      count++;
     }
 
-  return count > 1;
+  /* If we would only put out one conditional move, the other strategies
+     this pass tries are better optimized and will be more appropriate.
+     Some targets want to strictly limit the number of conditional moves
+     that are emitted, they set this through PARAM, we need to respect
+     that.  */
+  return count > 1 && count <= param;
 }
 
 /* Given a simple IF-THEN-JOIN or IF-THEN-ELSE-JOIN block, attempt to convert
@@ -3420,7 +3423,7 @@  noce_process_if_block (struct noce_if_info *if_info)
   if (!else_bb
       && HAVE_conditional_move
       && !HAVE_cc0
-      && bb_ok_for_noce_convert_multiple_sets (then_bb, if_info))
+      && bb_ok_for_noce_convert_multiple_sets (then_bb))
     {
       if (noce_convert_multiple_sets (if_info))
 	{
diff --git a/gcc/testsuite/gcc.dg/ifcvt-4.c b/gcc/testsuite/gcc.dg/ifcvt-4.c
index 319b583..0d1671c 100644
--- a/gcc/testsuite/gcc.dg/ifcvt-4.c
+++ b/gcc/testsuite/gcc.dg/ifcvt-4.c
@@ -1,4 +1,4 @@ 
-/* { dg-options "-fdump-rtl-ce1 -O2 --param max-rtl-if-conversion-insns=3" } */
+/* { dg-options "-fdump-rtl-ce1 -O2 --param max-rtl-if-conversion-insns=3 --param max-rtl-if-conversion-unpredictable-cost=100" } */
 /* { dg-additional-options "-misel" { target { powerpc*-*-* } } } */
 /* { dg-skip-if "Multiple set if-conversion not guaranteed on all subtargets" { "arm*-*-* hppa*64*-*-* visium-*-*" } }  */
 
diff --git a/gcc/testsuite/gcc.dg/ifcvt-5.c b/gcc/testsuite/gcc.dg/ifcvt-5.c
index 818099a..d2a9476 100644
--- a/gcc/testsuite/gcc.dg/ifcvt-5.c
+++ b/gcc/testsuite/gcc.dg/ifcvt-5.c
@@ -1,7 +1,8 @@ 
 /* Check that multi-insn if-conversion is not done if the override
-   parameter would not allow it.  */
+   parameter would not allow it.  Set the cost parameter very high
+   to ensure that the limiting factor is actually the count parameter.  */
 
-/* { dg-options "-fdump-rtl-ce1 -O2 --param max-rtl-if-conversion-insns=1" } */
+/* { dg-options "-fdump-rtl-ce1 -O2 --param max-rtl-if-conversion-insns=1 --param max-rtl-if-conversion-unpredictable-cost=200" } */
 
 typedef int word __attribute__((mode(word)));