https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102997
--- Comment #28 from rguenther at suse dot de <rguenther at suse dot de> --- On Mon, 8 Nov 2021, aldyh at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102997 > > --- Comment #27 from Aldy Hernandez <aldyh at gcc dot gnu.org> --- > (In reply to Richard Biener from comment #20) > > (In reply to hubicka from comment #19) > > > > The testcase would be > > > > > > void test () > > > { > > > int i; > > > if (test()) > > > i=0; > > > else > > > i=100; > > > for (;i<100 && bar ();i++) > > > do_work (); > > > } > > Updated test that compiles: > int check(); > int bar (); > void do_work (); > > void test () > { > int i; > if (check()) > i=0; > else > i=100; > for (;i<100 && bar ();i++) > do_work (); > } > > > > > > > > Here I think we can thread else path to return w/o any issues with loop > > > nests even with -Os. > > > > we fail this because > > > > // If we crossed a loop into an outer loop without crossing the > > // latch, this is just an early exit from the loop. > > if (loops_crossed == 1 > > && !crossed_latch > > && flow_loop_nested_p (exit->dest->loop_father, > > exit->src->loop_father)) > > return false; > > > > does not trigger (loops_crossed == 2), and we then run into > > > > > > if (loops_crossed) > > { > > cancel_thread (&path, "Path crosses loops"); > > return true; > > } > > > > but the loops_crossed stuff is old and not from me, not sure what that's > > useful for. We could "fix" the accounting with > > > > diff --git a/gcc/tree-ssa-threadupdate.c b/gcc/tree-ssa-threadupdate.c > > index 8aac733ac25..52ec22e85e1 100644 > > --- a/gcc/tree-ssa-threadupdate.c > > +++ b/gcc/tree-ssa-threadupdate.c > > @@ -2810,7 +2810,7 @@ jt_path_registry::cancel_invalid_paths > > (vec<jump_thread_edge *> &path) > > > > // If we crossed a loop into an outer loop without crossing the > > // latch, this is just an early exit from the loop. > > - if (loops_crossed == 1 > > + if (loops_crossed <= 2 > > && !crossed_latch > > && flow_loop_nested_p (exit->dest->loop_father, > > exit->src->loop_father)) > > return false; > > The above patch doesn't cause any difference in the generated code, at least > for -O2. I also don't see any more paths registered: The testcase was for -Os where it makes a difference. With -Os we do not perform loop header copying. > $ grep Registering.jump a.c.* > a.c.126t.thread1: [1] Registering jump thread: (2, 4) incoming edge; (4, 8) > nocopy; > a.c.126t.thread1: [2] Registering jump thread: (3, 4) incoming edge; (4, 7) > nocopy; > a.c.192t.dom3: [4] Registering jump thread: (6, 5) incoming edge; (5, 4) > joiner (4, 7) normal; > > > > > to also allow to thread through a loop path not crossing the latch but > > at least for the issue of "breaking loops" the loops_crossed stuff shouldn't > > be necessary. It might still prevent us from doing such transforms when > > it involves peeling. > > This loops crossing stuff is indeed quite old. Are you suggesting this as > well? > > - if (loops_crossed) > - { > - cancel_thread (&path, "Path crosses loops"); > - return true; > - } > > Which, BTW doesn't change the behavior for this test, but I'm happy to test if > you think it's not necessary. I'd have to go over the function and strip it down to what makes sense to me but then you know the thing is fragile, esp. with respect to the testsuite ;)
