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.
