On 10/21/2021 12:20 PM, Jeff Law wrote:
So if we're referring to those temporary const/copy propagations
"escaping" into Ranger, then I would fully expect that to cause
problems. Essentially they're path sensitive const/copy propagations
and may not be valid on all the paths through the CFG to the statement
where the propagation occurs
Yeah. disabling the global ranger should help, plus making sure you
don't use the ranger in the midst of the path sensitive changes.
I think we should first try to remove those temporary const/copy
propagations. As I noted in a different follow-up, I can't remember
if they were done as part of the original non-copying threader or if
they enabled further optimizations in the copying threader. If its
the former, then they can go away and that would be my preference.
I'll try to poke at that over the weekend.
OK. So those temporary propagations are still useful. Here's a simple
example (pr36550, but there are others):
[ ... ]
;; basic block 3, loop depth 0, count 536870912 (estimated locally),
maybe hot
;; prev block 2, next block 4, flags: (NEW, VISITED)
;; pred: 2 [50.0% (guessed)] count:536870912 (estimated
locally) (TRUE_VALUE,EXECUTABLE)
# VUSE <.MEM_10>
_20 = *argv_12(D);
if (_20 != 0B)
goto <bb 4>; [70.00%]
else
goto <bb 7>; [30.00%]
;; succ: 4 [70.0% (guessed)] count:375809640 (estimated
locally) (TRUE_VALUE,EXECUTABLE)
;; 7 [30.0% (guessed)] count:161061272 (estimated
locally) (FALSE_VALUE,EXECUTABLE)
[ ... ]
;; basic block 7, loop depth 0, count 536763538 (estimated locally),
maybe hot
;; prev block 6, next block 8, flags: (NEW, VISITED)
;; pred: 3 [30.0% (guessed)] count:161061272 (estimated
locally) (FALSE_VALUE,EXECUTABLE)
;; 4 [always] count:375809640 (estimated locally)
(FALLTHRU,EXECUTABLE)
# argv_32 = PHI <argv_12(D)(3), argv_21(4)>
# bug_33 = PHI <bug_16(D)(3), bug_22(4)>
# VUSE <.MEM_10>
_3 = *argv_32;
if (_3 == 0B)
goto <bb 9>; [18.09%]
else
goto <bb 8>; [81.91%]
;; succ: 9 [18.1% (guessed)] count:97100524 (estimated
locally) (TRUE_VALUE,EXECUTABLE)
;; 8 [81.9% (guessed)] count:439663014 (estimated
locally) (FALSE_VALUE,EXECUTABLE)
So when we reach block 7 directly from block 3 we'll have _20 =
*argv_12(MEM_10) = _20 in the expression hash table and _20 = 0 in the
const/copies table.
The first statement in block #7 loads *argv_32. Without the temporary
propagation we'll lookup *argv_32(MEM_10) in the hash table which misses
and we're unable to thread the subsequent conditional in block #7.
With the temporary propagation instead of looking up *argv_32(MEM_10),
we propagate argv_12 for argv32 and lookup *argv32(MEM_10) which hits
with the LHS value _20 which we then lookup in const/copies getting the
constant 0. Thus we know *argv_32 will have the value 0 when block 7
is reached directly via block 3. That in turn allows us to know that
the conditional at the end of block #7 is threadable when reached via
block #3.
In this particular testcase we end up getting a failure because...
drumroll.... the failure to thread 3->7->9 leaves an infeasible path in
the CFG which in turn causes a bogus Wuninitialized warning.
Instead of actually propagating the arguments into the statement, we
could revamp the hashing bits so that they used the data from the
const/copy tables. That would avoid twiddling the IL. Though I'm
still not sure how the IL twiddling could be escaping at this point.
jeff
