On Wed, Apr 4, 2012 at 9:15 PM, William J. Schmidt <wschm...@linux.vnet.ibm.com> wrote: > On Wed, 2012-04-04 at 15:08 +0200, Richard Guenther wrote: >> On Wed, Apr 4, 2012 at 2:35 PM, William J. Schmidt >> <wschm...@linux.vnet.ibm.com> wrote: >> > On Wed, 2012-04-04 at 13:35 +0200, Richard Guenther wrote: >> >> On Tue, Apr 3, 2012 at 10:25 PM, William J. Schmidt >> >> <wschm...@linux.vnet.ibm.com> wrote: >> >> > >> >> > >> >> > On Wed, 2012-03-28 at 15:57 +0200, Richard Guenther wrote: >> >> >> On Tue, Mar 6, 2012 at 9:49 PM, William J. Schmidt >> >> >> <wschm...@linux.vnet.ibm.com> wrote: >> >> >> > Hi, >> >> >> > >> >> >> > This is a re-post of the patch I posted for comments in January to >> >> >> > address http://gcc.gnu.org/bugzilla/show_bug.cgi?id=18589. The patch >> >> >> > modifies reassociation to expose repeated factors from __builtin_pow* >> >> >> > calls, optimally reassociate repeated factors, and possibly >> >> >> > reconstitute >> >> >> > __builtin_powi calls from the results of reassociation. >> >> >> > >> >> >> > Bootstrapped and passes regression tests for powerpc64-linux-gnu. I >> >> >> > expect there may need to be some small changes, but I am targeting >> >> >> > this >> >> >> > for trunk approval. >> >> >> > >> >> >> > Thanks very much for the review, >> >> >> >> >> >> Hmm. How much work would it be to extend the reassoc 'IL' to allow >> >> >> a repeat factor per op? I realize what you do is all within what >> >> >> reassoc >> >> >> already does though ideally we would not require any GIMPLE IL changes >> >> >> for building up / optimizing the reassoc IL but only do so when we >> >> >> commit >> >> >> changes. >> >> >> >> >> >> Thanks, >> >> >> Richard. >> >> > >> >> > Hi Richard, >> >> > >> >> > I've revised my patch along these lines; see the new version below. >> >> > While testing it I realized I could do a better job of reducing the >> >> > number of multiplies, so there are some changes to that logic as well, >> >> > and a couple of additional test cases. Regstrapped successfully on >> >> > powerpc64-linux. >> >> > >> >> > Hope this looks better! >> >> >> >> Yes indeed. A few observations though. You didn't integrate >> >> attempt_builtin_powi >> >> with optimize_ops_list - presumably because it's result does not really >> >> fit >> >> the single-operation assumption? But note that undistribute_ops_list and >> >> optimize_range_tests have the same issue. Thus, I'd have prefered if >> >> attempt_builtin_powi worked in the same way, remove the parts of the >> >> ops list it consumed and stick an operand for its result there instead. >> >> That should simplify things (not having that special powi_result) and >> >> allow for multiple "powi results" in a single op list? >> > >> > Multiple powi results are already handled, but yes, what you're >> > suggesting would simplify things by eliminating the need to create >> > explicit multiplies to join them and the cached-multiply results >> > together. Sounds reasonable on the surface; it just hadn't occurred to >> > me to do it this way. I'll have a look. >> > >> > Any other major concerns while I'm reworking this? >> >> No, the rest looks fine (you should not need to repace >> -fdump-tree-reassoc-details >> with -fdump-tree-reassoc1-details -fdump-tree-reassoc2-details in the first >> testcase). > > Unfortunately this seems to be necessary if I name the two passes > "reassoc1" and "reassoc2". If I try to name both of them "reassoc" I > get failures in other tests like gfortran.dg/reassoc_4, where > -fdump-tree-reassoc1 doesn't work. Unless I'm missing something > obvious, I think I need to keep that change.
Hm, naming them "reassoc1" and "reassoc2" is a hack. Naming both "reassoc" will not trigger re-naming them to reassoc1 and reassoc2 I think. How ugly. Especially that -fdump-tree-reassoc will no longer work. Maybe instead of using two pass structs resort to using the existing hack with using first_pass_instance and TODO_mark_first_instance. > Frankly I was surprised and relieved that there weren't more tests that > used the generic -fdump-tree-reassoc. > > Thanks, > Bill >> >> Thanks, >> Richard. >> >> > Thanks, >> > Bill >> >> >> >> Thanks, >> >> Richard. >> >> >> > >> > >> > >
