This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RFC: fix think-o in tree-ssa-loop-ivopts.c


Still working on PR42505, I found another problem in the ivopts cost computation. This is apparently not the cause of the regression in PR42505 either, but there's clearly something wrong here.....

This patch

http://gcc.gnu.org/ml/gcc-patches/2009-05/msg01579.html

which was committed as r139821 includes this bit:

/* Having offset does not affect runtime cost in case it is added to
symbol, but it increases complexity. */
if (offset)
cost.complexity++;
- cost.cost += n_sums * add_cost (TYPE_MODE (ctype), speed);
- return cost;
+ cost.cost += add_cost (TYPE_MODE (ctype), speed);
+
+ aratio = ratio > 0 ? ratio : -ratio;
+ if (aratio != 1)
+ cost.cost += multiply_by_cost (aratio, TYPE_MODE (ctype), speed);
fallback:
{

The "cost" is never referred to again in the fallback code, so it seems like deleting the return statement was an unintended mistake.


I quickly put together the attached patch with the obvious fix and re-ran CSiBE on x86-64 native.... and I found that code size went up, giving back about half the improvement I got from adjusting the iv setup costs for -Os.

So, I'm wondering: Eric, were the runtime speed improvements you claimed you saw here

http://gcc.gnu.org/ml/gcc-patches/2009-05/msg01858.html

actually present in the checked-in version of the patch, where the result of the fancy cost computation is simply discarded, or in some other version of the patch that implemented what I presume was the intended behavior? Do we know for sure that the fancy cost computation actually even an improvement over the fallback case?

TBH, I don't really have time to work on re-checking x86 benchmark results myself.... I'm just trying to grok the costs code so I can fix PR42505 for one of our ARM customers. ;-)

-Sandra


2010-06-09 Sandra Loosemore <sandra@codesourcery.com>


	gcc/
	* tree-ssa-loop-ivopts.c (get_computation_cost_at): Return the
	computed cost.


Index: gcc/tree-ssa-loop-ivopts.c
===================================================================
--- gcc/tree-ssa-loop-ivopts.c	(revision 160471)
+++ gcc/tree-ssa-loop-ivopts.c	(working copy)
@@ -3875,6 +3875,7 @@ get_computation_cost_at (struct ivopts_d
   aratio = ratio > 0 ? ratio : -ratio;
   if (aratio != 1)
     cost.cost += multiply_by_cost (aratio, TYPE_MODE (ctype), speed);
+  return cost;
 
 fallback:
   if (can_autoinc)

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]