Re: [PATCH] Improve backwards threading

[Richard Biener <rguenther at suse dot de>]

On Fri, 5 Aug 2016, Jeff Law wrote:

> On 08/05/2016 07:36 AM, Richard Biener wrote:
> > @@ -560,6 +562,14 @@ fsm_find_control_statement_thread_paths
> >           edge taken_edge = profitable_jump_thread_path (path, bbi, name,
> > arg);
> >           if (taken_edge)
> >             {
> > +             /* If the taken edge is a loop entry avoid mashing two
> > +                loops into one with multiple latches by splitting
> > +                the edge.  This only works if that block won't become
> > +                a latch of this loop.  */
> > +             if ((bb_loop_depth (taken_edge->src)
> > +                  < bb_loop_depth (taken_edge->dest))
> > +                 && ! single_succ_p (bbi))
> > +               split_edge (taken_edge);
> >               if (bb_loop_depth (taken_edge->src)
> >                   >= bb_loop_depth (taken_edge->dest))
> >                 convert_and_register_jump_thread_path (path, taken_edge);
> > 
> > note you need the PR72772 fix to trigger all this.
> I'm a little confused here.  In the case where the taken edge goes into a
> deeper loop nest you're splitting the edge -- to what end?  The backwards
> threader isn't going to register that jump thread.  So if this is fixing
> something, then we've got the fix in the wrong place.

Note that the case is not going "into" a deeper loop nest but as the
threading path ends at taken_edge it is threading to a loop header of
an inner loop.  And yes, the fix doesn't work (not sure how I thought
it could).  But the condition also doesn't make sense to me and we
miss a guard for the case I added a comment for - generating wrong-code
because loop meta data is wrong after the transform (I think this
may not only apply to the niter upper bound but also things like
safelen).

> FWIW, I the anonymous name fix ought to go in separately, it looks like the
> right thing to do independent of this stuff.

I have applied that part now.

I'm not sure what to do with the pre-header creation fix (PR72772) now,
but I am considering to put that into the tree regardless of the
"fallout" (a few FAILs for transforms we no longer perform).  I spent
half a week now but am quite lost with the threading cases (which
I think are latent issues).

Richard.