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.
