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? Thanks, Bill > > Thanks, > Richard. >
