This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [parloops, PR83126], Use cached affine_ivs canonicalize_loop_ivs
On Wed, 21 Mar 2018, Tom de Vries wrote:
> On 03/12/2018 01:14 PM, Richard Biener wrote:
> > On Thu, 22 Feb 2018, Tom de Vries wrote:
> >
> > > Hi,
> > >
> > > this patch fixes an ICE in the parloops pass.
> > >
> > > The ICE (when compiling the test-case in attached patch) follows from the
> > > fact
> > > that here in gen_parallel_loop the call to canonicalize_loop_ivs fails to
> > > "base all the induction variables in LOOP on a single control one":
> > > ...
> > > /* Base all the induction variables in LOOP on a single control one.*/
> > > canonicalize_loop_ivs (loop, &nit, true);
> > > ...
> > >
> > > This is caused by the fact that for one of the induction variables,
> > > simple_iv
> > > no longer returns true (as was the case at the start of
> > > gen_parallel_loop).
> > > This seems to be triggered by the fact that during loop_version we call
> > > scev_reset (although I'm not sure why when recalculating scev info we're
> > > not
> > > reaching the same conclusion as before).
> > I guess the real reason is that canonicalize_loop_ivs calls
> > create_iv first which in the parloop case with bump-in-latch true
> > and then incrementally re-writes PHIs, eventually wrecking other
> > PHIs SCEV. In this case the 2nd PHIs evolution depends on the
> > first one but the rewritten ones depend on the new IV already.
> >
> > So the better fix (maybe not 100% enough) would be to re-organize
> > canonicalize_loop_ivs to first do analysis of all PHIs and then
> > rewrite them all.
> >
>
> Focusing on the re-organize first, is this sort of what you had in mind?
Yes, though not sure why you need to have a hash-map here, just pushing
to a vector of PHIs would work, no? Or alternatively a bitmap
of SSA versions so you have a way to match the gphi iterator with
the set of IVs. But the vector should be sorted in PHI order so
just traversing the PHIs looking for the "next" PHI in the vector
would work I guess.
Does it help with the original issue btw?
Richard.
> Bootstrapped and reg-tested on x86_64.
>
> Thanks,
> - Tom
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)