[Bug middle-end/102997] [12 Regression] 45% 454.calculix regression with LTO+PGO -march=native -Ofast on Zen since r12-4526-gd8edfadfc7a9795b65177a50ce44fd348858e844

aldyh at gcc dot gnu.org gcc-bugzilla@gcc.gnu.org
Mon Nov 8 15:34:17 GMT 2021


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:

$ 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.


More information about the Gcc-bugs mailing list