diff mbox

[PR62631] Return shiftadd cost only when it's smaller than normal one

Message ID 000c01cfc81e$87780e80$96682b80$@arm.com
State New
Headers show

Commit Message

Bin Cheng Sept. 4, 2014, 8:59 a.m. UTC 
Hi,
As reported by pr62631, case ivopts-lt-2.c is failed on sparc64.  The root
cause is shiftadd_cost calculated on sparc64 is huge which prevents gcc from
choosing the wanted candidate.  The problem with IVOPT is it always assumes
that shiftadd_cost is smaller than the normal cost, while it's not true in
this extreme case.  This patch checks if the shiftadd_cost is smaller before
returning it.  The patch also cleans code indent in context.

One question is left here.
Why >100 cost is calculated for shiftadd on sparc64?  In final assembly
code, the computation is implemented by separated shift and add
instructions.  Probably sparc64 should return cost no larger than
cost(shift)+cost(add) in the first place?.

Bootstrap and tested on x86_64/cortex-m3, the case is fixed on sparc64 now.
Is it OK?

Thanks,
bin


2014-09-04  Bin Cheng  <bin.cheng@arm.com>

	* tree-ssa-loop-ivopts.c (force_expr_to_var_cost): Return cost
	calculated by get_shiftadd_cost only when it is smaller.  Fix
	code indent.

Comments

Richard Biener Sept. 4, 2014, 1:16 p.m. UTC | #1 
On Thu, Sep 4, 2014 at 10:59 AM, Bin Cheng <bin.cheng@arm.com> wrote:
> Hi,
> As reported by pr62631, case ivopts-lt-2.c is failed on sparc64.  The root
> cause is shiftadd_cost calculated on sparc64 is huge which prevents gcc from
> choosing the wanted candidate.  The problem with IVOPT is it always assumes
> that shiftadd_cost is smaller than the normal cost, while it's not true in
> this extreme case.  This patch checks if the shiftadd_cost is smaller before
> returning it.  The patch also cleans code indent in context.
>
> One question is left here.
> Why >100 cost is calculated for shiftadd on sparc64?  In final assembly
> code, the computation is implemented by separated shift and add
> instructions.  Probably sparc64 should return cost no larger than
> cost(shift)+cost(add) in the first place?.
>
> Bootstrap and tested on x86_64/cortex-m3, the case is fixed on sparc64 now.
> Is it OK?

So is the IVOPTs code just checking if the target returned
sth > cost(shift) + cost(add)?  Then I think we should fix the target
instead.

Richard.

> Thanks,
> bin
>
>
> 2014-09-04  Bin Cheng  <bin.cheng@arm.com>
>
>         * tree-ssa-loop-ivopts.c (force_expr_to_var_cost): Return cost
>         calculated by get_shiftadd_cost only when it is smaller.  Fix
>         code indent.
diff mbox

Patch

Index: gcc/tree-ssa-loop-ivopts.c
===================================================================
--- gcc/tree-ssa-loop-ivopts.c	(revision 214674)
+++ gcc/tree-ssa-loop-ivopts.c	(working copy)
@@ -3730,20 +3730,23 @@  force_expr_to_var_cost (tree expr, bool speed)
     case NEGATE_EXPR:
       cost = new_cost (add_cost (speed, mode), 0);
       if (TREE_CODE (expr) != NEGATE_EXPR)
-        {
-          tree mult = NULL_TREE;
-          comp_cost sa_cost;
-          if (TREE_CODE (op1) == MULT_EXPR)
-            mult = op1;
-          else if (TREE_CODE (op0) == MULT_EXPR)
-            mult = op0;
+	{
+	  tree mult = NULL_TREE;
+	  comp_cost sa_cost;
+	  if (TREE_CODE (op1) == MULT_EXPR)
+	    mult = op1;
+	  else if (TREE_CODE (op0) == MULT_EXPR)
+	    mult = op0;
 
-          if (mult != NULL_TREE
-              && cst_and_fits_in_hwi (TREE_OPERAND (mult, 1))
-              && get_shiftadd_cost (expr, mode, cost0, cost1, mult,
-                                    speed, &sa_cost))
-            return sa_cost;
-        }
+	  if (mult != NULL_TREE
+	      && cst_and_fits_in_hwi (TREE_OPERAND (mult, 1))
+	      && get_shiftadd_cost (expr, mode, cost0, cost1, mult,
+				    speed, &sa_cost)
+	      && compare_costs (sa_cost,
+				add_costs (cost,
+					   add_costs (cost0, cost1))) < 0)
+	    return sa_cost;
+	}
       break;
 
     CASE_CONVERT: