[PATCH] D75799: [JumpThreading] Don't create PHI nodes with "is.constant" values

Fri Mar 13 05:06:51 PDT 2020

void added a comment.

In D75799#1921129 <https://reviews.llvm.org/D75799#1921129>, @lebedev.ri wrote:

> In D75799#1920869 <https://reviews.llvm.org/D75799#1920869>, @void wrote:
>
> > In D75799#1920392 <https://reviews.llvm.org/D75799#1920392>, @lebedev.ri wrote:
> >
> > > Can you post standalone example on godbolt that shows this link failure without this patch please?
> >
> >
> > I'm not sure what that will achieve. It would include all of the Linux kernel as part of the linking process, and it's only one file (the one I reduced the testcase from) that has the `__bad_copy_from()` reference still in it. Perhaps I'm missing something?
>
>
> I personally still don't understand *why* after this transform we fail to DCE. Is this pass ordering issue? Heuristics issue?

I gave a fairly lengthy explanation above. The test case is also instructive, and would be much easier to see what's happening, to tell the truth.

It's not about pass ordering. It's about how jump threading splits a PHI node into two parts: one with a constant value and one with a non-constant value. This is fine in normal situations, but the `is.constant` intrinsic is not normal, at least not in the way one thinks instructions are. It's meant to determine if the variable *at the point it's called* is constant or non-constant. If there are two values combined by a PHI node with one constant and the other non-constant, it's not appropriate to split the PHI apart into constant and non-constant paths, because then what would normally be DCE'd (like the call to `__bad_copy_from`) won't be, because there's one path where the `is.constant` is `true`, thus keeping the call to `__bad_copy_from()` around.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75799/new/

https://reviews.llvm.org/D75799