Message ID | 4C0FCE4D.7010005@codesourcery.com |
---|---|
State | New |
Headers | show |
> The "cost" is never referred to again in the fallback code, so it seems > like deleting the return statement was an unintended mistake. Indeed, how embarassing. :-( This affects 4.5.x as well. > 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? Probably in both versions, at least partially. The main effect (saving 1 register for loops with non-constant iteration origin) is always present. > Do we know for sure that the fancy cost computation actually even an > improvement over the fallback case? The "fancy" computation was an improvement, yes. > 2010-06-09 Sandra Loosemore <sandra@codesourcery.com> > > gcc/ > * tree-ssa-loop-ivopts.c (get_computation_cost_at): Return the > computed cost. I'd add "again". Thanks for spotting this.
On Wed, Jun 9, 2010 at 9:59 PM, Eric Botcazou <ebotcazou@adacore.com> wrote: >> The "cost" is never referred to again in the fallback code, so it seems >> like deleting the return statement was an unintended mistake. > > Indeed, how embarassing. :-( This affects 4.5.x as well. > >> 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? > > Probably in both versions, at least partially. The main effect (saving 1 > register for loops with non-constant iteration origin) is always present. > >> Do we know for sure that the fancy cost computation actually even an >> improvement over the fallback case? > > The "fancy" computation was an improvement, yes. > >> 2010-06-09 Sandra Loosemore <sandra@codesourcery.com> >> >> gcc/ >> * tree-ssa-loop-ivopts.c (get_computation_cost_at): Return the >> computed cost. > > I'd add "again". Thanks for spotting this. The patch is ok if it passes bootstrap and regtest. It's also ok for the branch after a while. Thanks, Richard. > -- > Eric Botcazou >
> The patch is ok if it passes bootstrap and regtest. It's also ok for the > branch after a while. Thanks. Sandra, I can take care of this if you wish.
Eric Botcazou wrote: >> The patch is ok if it passes bootstrap and regtest. It's also ok for the >> branch after a while. > > Thanks. Sandra, I can take care of this if you wish. That would be great. I've gotten sucked into another high-priority project and wouldn't have time to work on it until next week, probably. -Sandra
> That would be great. I've gotten sucked into another high-priority project > and wouldn't have time to work on it until next week, probably. OK, bootstrapped/regtested on x86_64-suse-linux, applied on the mainline.
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)