Jump Theading/GVN bug

[Daniel Berlin]

On Mon Feb 23 2015 at 7:38:37 AM Rafael Espíndola <
rafael.espindola at gmail.com> wrote:

> >> This instruction doesn’t appear to be well formed. However, one could
> >> argue that it is – formally %2 def does dominate all %2 uses because
> both
> >> are located in an unreachable block, so every path to the use is passing
> >> through the def (because there are 0 paths like that).
> >>
> >> That is likely the reason why –verify doesn’t complain about it.
> >
> >
> > Why is jump-threading creating this funky instruction?
>
> It is well formed. I agree with Hal: it might be a quality improvement
> to generate tidier IR from jump-threading, but GVN has to handle
> whatever input passes the verifier.
>

Then, IMHO, we should start the long process of making forward-unreachable
code not acceptable except inside a single pass (IE at the end of each
pass, there should be no unreachable code).

There are a very very small number of passes that generate such code, and
it's not that difficult to fix them to make them not do that.

On the other hand, having *all* passes required to handle pretty much
garbage, as long as it's unreachable, has a pretty high cost.

GVN already spends a bunch of code and time trying to deal with the
weirdness of unreachable code, as do a lot of other passes (For reference,
5-10% of GVN's testcases deal with making sure it doesn't crash on
unreachable code, depending on how you count)

At least a few other passes i've dealt with in the past week also go out of
their way, after dealing with normal code, to deal  with unreachable code
just to avoid crashing themselves later. So we are wasting compile time and
maintenance burden on it too.

Worse, they almost all have bugs in how they handle it (I found two this
week while using a utility class for something).
Fixing them all to handle unreachable code "properly" (since there is no
real proper thing to do in most cases except treat it as radioactive parts
of the input that you shouldn't really touch, but still have to do
something with,  since phi nodes may have unreachable predecessors, etc),
IMHO, brings no appreciable benefit to the compiler, particularly when the
alternative is "make it illegal".


> The GVN crashes reduces to just running GVN in
>
> define i8 @foo(i8* %x) {
> bb0:
>   br label %bb2
> bb1:
>   %tmp1 = getelementptr inbounds i8* %tmp1, i64 1
>   br i1 false, label %bb2, label %bb1
> bb2:
>   %tmp2 = phi i8* [ %tmp1, %bb1 ], [ %x, %bb0 ]
>   %tmp3 = load i8* %tmp2, align 1
>   ret i8 %tmp3
> }
>
> Cheers,
> Rafael
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150223/c4f8a566/attachment.html>