[LLVMdev] Jump Theading/GVN bug - moving discussion to llvm-dev

[Daniel Berlin]

On Mon, Feb 23, 2015 at 11:05 PM, Chandler Carruth <chandlerc at google.com>
wrote:

> Will try to reply to the larger thread again soon, but a quick reply on a
> bit of a tangent...
>
> On Mon, Feb 23, 2015 at 8:45 PM, Hal Finkel <hfinkel at anl.gov> wrote:
>
>>  2. Should unreachable code be allowed to contain nonsense (like
>> instructions that depend on themselves)? To this, I believe the answer is
>> no. We currently permit this, and I think that a lot of the bugs regarding
>> unreachable code some from this. I've yet to hear a good argument why, for
>> example, JumpThreading must produce self-referential instructions in
>> trivially-dead regions.
>>
>
> I could easily see tightening the rules on what passes the verifier within
> unreachable code to try to make it substantially cheaper, easier, and less
> error prone to handle unreachable code. It seems likely that there are good
> pragmatic compromises here that would actually cost nothing.
>


One such compromise (which you didn't seem fond of) was that "unreachable
blocks have to contain nothing but a terminator".

Earlier, you said "The code which is most likely to create unreachable code
is similarly likely to destroy the analysis information we would use to
remove it cheaply. " and "When transforming the IR we must never forget
about regions we have made unreachable. "

I would like to challenge this assertion with actual evidence:
Right now the state of the world is actually "the vast majority of llvm
passes try to clean up after themselves, try not to create weird
unreachable code".

GVN removes dead blocks that replacement of values/equality propagation
creates
SCCP deletes all instructions in dead blocks except terminators (because it
preserves CFG)
ADCE does the same (it never removes terminators)
DCE the same
IPSCCP actually removes dead blocks, because it does not preserve CFG
JumpThreading removes unreachable blocks on input but not output (and in
fact, as you'll see, it's the only pass that seems to really be a problem)
SROA cleans up unreachable code it generates in it's utility class
SimplifyCFG removes unreachable blocks
CorrelatedValuePropagation does the same through a utility (it calls
constantfoldterminator, which in turn takes great pains to clean up the CFG
properly)
etc
I didn't look at the loop passes, but outside of jump threading, it appears
there is little work to be done to be in a state where things clean up
after themselves.

That is, *right* now, we know for at least a ton of the passes and their
analysis, it is not "destroying the analysis information we would use to
remove it" because they are in fact removing it.
They already "do not forget about regions they have made unreachable",
because they already clean up after themselves.

For giggle, i stopped two of them that i happened to know pretty well (GVN,
CorrelatedValuePropagation) from removing unreachable blocks, just leaving
them unreachable with instructions still in them.
While it does nothing to cause verifier crashes, it causes a *bunch* of
brand new crashes *in* passes in the standard pass ordering, which it
wouldn't if we were really living in a world where passes were really
handling unreachable code properly already.

Regardless of what the outcome is (and i'm pretty clearly on one side), we
shouldn't go half-way. Right now we are already in a world, where, because
we have some passes cleaning up and some not,we have passes that can't
handle unreachable code as input, and some that can.
So if you change the pass order, and those passes do something, you get
wonderful crashes.
Either we should be testing passes with unreachable code as input, or
expecting their not to be any.
(and the side-issue of "in a world where anything can be in an unreachable
block, all of this cleanup and carefulness is a waste of time")


Obvious example is, as you highlight, self-referential non-phi instructions
> could easily just be rejected, and asserted during construction by the IR
> builder.
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150224/b9c50d08/attachment.html>