Re: [PATCH] Strength reduction

[Richard Guenther <rguenther at suse dot de>]

On Tue, 26 Jun 2012, William J. Schmidt wrote:

> On Tue, 2012-06-26 at 15:06 +0200, Richard Guenther wrote:
> > On Mon, 25 Jun 2012, William J. Schmidt wrote:
> > 
> > > Here's a new version of the main strength reduction patch, addressing
> > > previous comments.  A couple of quick notes:
> > > 
> > > * I opened PR53773 and PR53774 for the cases where commutative
> > > operations were encountered with a constant in rhs1.  This version of
> > > the patch still has the gcc_asserts in place to catch those cases, but
> > > I'll plan to remove those once the patch is approved.
> > > 
> > >  * You previously asked:
> > > 
> > > >>
> > > >> +static slsr_cand_t
> > > >> +base_cand_from_table (tree base_in)
> > > >> +{
> > > >> +  slsr_cand mapping_key;
> > > >> +
> > > >> +  gimple def = SSA_NAME_DEF_STMT (base_in);
> > > >> +  if (!def)
> > > >> +    return (slsr_cand_t) NULL;
> > > >> +
> > > >> +  mapping_key.cand_stmt = def;
> > > >> +  return (slsr_cand_t) htab_find (stmt_cand_map, &mapping_key);
> > > >>
> > > >> isn't that reachable via the base-name -> chain mapping for base_in?
> > > 
> > > I had to review this a bit, but the answer is no.  If you look at one of
> > > the algebraic manipulations in create_mul_ssa_cand as an example,
> > > base_in corresponds to Y.  base_cand_from_table is looking for a
> > > candidate that has Y for its LHS.  The base-name -> chain mapping is
> > > used to find all candidates that have B as the base_name.
> > > 
> > >  * I added a detailed explanation of what's going on with legal_cast_p.
> > > Hopefully this will be easier to understand now.
> > > 
> > > I've bootstrapped this on powerpc64-unknown-linux-gnu with three new
> > > regressions (for which I opened the two bug reports).  Ok for trunk
> > > after removing the asserts?
> > 
> > Ok.  Please keep an eye on possible fallout.
> 
> Yep, will do.
> 
> > 
> > One more question - you put the pass inbetween VRP and DOM, any
> > reason to not put it after DOM/phi_only_cprop which perform CSE?
> > Thus, does strength-reduction expose CSE opportunities?
> 
> It can introduce copies in some circumstances, which DOM will propagate.
> That was the main reason for putting it there, IIRC.  I originally
> placed it after DOM so it would be in a copy-free environment, but it
> ended up leaving some garbage behind that way.
> 
> An alternative would be to explicitly propagate any introduced copies
> and move the pass later.  I don't recall offhand if there were other
> reasons besides copy propagation for moving the pass -- I looked back
> through my notes and apparently didn't record anything about it at the
> time.

Fair enough - the pass pipeline after loop optimizers is not thoroughly
thought through anyway.

Thanks,
Richard.