Re: [RFC] More jump threading restrictions in the presence of loops.

[Jeff Law <jeffreyalaw@gmail.com>]

On 10/4/2021 7:36 AM, Aldy Hernandez wrote:
> Note that none of the other tests have been adjusted so it'll likely 
> result in multiple threading regressions.
Yea ;-)  So maybe we first focus on how those affect visium since that's 
what started us down this path.

The original test of regressions for visium were:
visium-sim: gcc.c-torture/execute/960218-1.c   -Os  (test for excess errors)
visium-sim: gcc.c-torture/execute/961125-1.c   -O3 -fomit-frame-pointer 
-funroll-loops -fpeel-loops -ftracer -finline-functions  (test for 
excess errors)
visium-sim: gcc.c-torture/execute/961125-1.c   -O3 -g  (test for excess 
errors)
visium-sim: gcc.c-torture/execute/pending-4.c   -O1  (test for excess 
errors)
visium-sim: gcc.c-torture/execute/pr58209.c   -O2  (test for excess errors)
visium-sim: gcc.c-torture/execute/pr58209.c   -O2 -flto 
-fno-use-linker-plugin -flto-partition=none  (test for excess errors)
visium-sim: gcc.c-torture/execute/pr58209.c   -O2 -flto 
-fuse-linker-plugin -fno-fat-lto-objects  (test for excess errors)
visium-sim: gcc.c-torture/execute/pr58209.c   -O3 -g  (test for excess 
errors)
visium-sim: gcc.c-torture/execute/pr68911.c   -O1  (test for excess errors)

I'm pretty confident these are all cases where we previously threaded 
one or more jumps and as a result we were able to remove all calls to 
abort ().   Your change doesn't affect any of those :(

So probably the first thing to do is get a patch that fixes one or more 
of those failures *or* make a determination that one or more of those 
tests are things we do not want to optimize.   The one we had previously 
looked at (960218-1)  had a thread path from inside the loop to a point 
outside the loop.  My sense is we probably want to allow that.

And just in case it got lost.  Here's the analysis on 960218-1 on visium:

We've got this going into ethread:

int f (int x)
{
   int D.1420;
   int a;

;;   basic block 2, loop depth 0, maybe hot
;;    prev block 0, next block 3, flags: (NEW, REACHABLE, VISITED)
;;    pred:       ENTRY (FALLTHRU,EXECUTABLE)
   a_4 = ~x_3(D);
   goto <bb 4>; [INV]
;;    succ:       4 (FALLTHRU,EXECUTABLE)

;;   basic block 3, loop depth 1, maybe hot
;;    prev block 2, next block 4, flags: (NEW, REACHABLE, VISITED)
;;    pred:       4 (TRUE_VALUE,EXECUTABLE)
   gl = a_1;
;;    succ:       4 (FALLTHRU,DFS_BACK,EXECUTABLE)

;;   basic block 4, loop depth 1, maybe hot
;;    prev block 3, next block 5, flags: (NEW, REACHABLE, VISITED)
;;    pred:       2 (FALLTHRU,EXECUTABLE)
;;                3 (FALLTHRU,DFS_BACK,EXECUTABLE)
   # a_1 = PHI <a_4(2), 0(3)>
   if (a_1 != 0)
     goto <bb 3>; [INV]
   else
     goto <bb 5>; [INV]
;;    succ:       3 (TRUE_VALUE,EXECUTABLE)
;;                5 (FALSE_VALUE,EXECUTABLE)

;;   basic block 5, loop depth 0, maybe hot
;;    prev block 4, next block 1, flags: (NEW, REACHABLE, VISITED)
;;    pred:       4 (FALSE_VALUE,EXECUTABLE)
   return;
;;    succ:       EXIT

}


There's a pretty obvious jump thread in there.  3->4->5.  We used to 
allow this, but it's currently being rejected and I'm not sure it should.

In simplest terms we're threading from inside the loop, through the 
latch to a point outside the loop.  At first glance, that seems safe.

960218-1.c, compiled with -Os

int gl;

g (x)
{
   gl = x;
   return 0;
}

f (x)
{
   int a = ~x;
   while (a)
     a = g (a);
}

main ()
{
   f (3);
   if (gl != -4)
     abort ();
   exit (0);
}


Ideally in the final assembly there would be no calls to abort since it 
can be determined at compile-time that the call can never happen.

jeff