Fix the case of an unreachable entry in LocalGraph #6048
Conversation
|
Unfortunately this is not enough either... it's possible depending on unreachability here is not the best idea. edit: there may be a loop later on, that is in effect unreachable even though it has an entry from itself |
|
A proper analysis of unreachability may not be worth it, as it would add overhead that could easily overtake the benefits of this optimization. So I am considering just reverting the previous PR, sadly. |
|
I pushed a proper fix, but still need to measure if the slowdown is worth it. |
|
Sadly this does make us a little slower. Just 1-2% or so, but as the whole point here was an optimization, it makes it not worthwhile. I'll experiment with the other idea of relaxing those assertions. |
|
I pushed a version that relaxes the assertion as @tlively suggested, and am fuzzing it now. It may be the best option here. |
|
This passed 12K fuzzer iterations, so I think it is stable, and is the best solution we have AFAICT. |
Followup to #6048, we did not handle nondefaultable tuples because of this.
…ssembly#6048) Followup to WebAssembly#6046 - the fuzzer found we missed handling the case of the entry itself being unreachable, or of an unreachable loop later. Properly identifying unreachable code requires a flow analysis, unfortunately, so this PR gives up on that and instead allows LocalGraph to be imprecise in unreachable code. That avoids adding any overhead, but does mean the IR may be slightly confusing when debugging. It does not have any optimization downsides, however, as it only affects unreachable code. Also add a dump() impl in that file which helps debugging.
Followup to WebAssembly#6048, we did not handle nondefaultable tuples because of this.
Followup to #6046 - the fuzzer found we missed handling the case of the entry itself
being unreachable and there being a loop after it, see code and testcase.
Also add a "dump" impl in that file which helps debugging.