[PATCH] D64963: Add a pass to lower is.constant intrinsics

[Bill Wendling via Phabricator via llvm-commits]

void marked an inline comment as done.
void added inline comments.


================
Comment at: lib/Transforms/Scalar/LowerIsConstantIntrinsic.cpp:44
+  if (Exp) {
+    II->replaceAllUsesWith(Exp);
+    II->eraseFromParent();
----------------
joerg wrote:
> void wrote:
> > joerg wrote:
> > > void wrote:
> > > > joerg wrote:
> > > > > RAUW is not good enough here. We want to have at least replaceAndRecursivelySimplify, but even that is not good enough as it still leaves conditional branches on constant conditions. D62025 depended on SimplifyCFG for that, but that doesn't work for the -O0 chain. That said, is there a reason for not handling that in InstructionSimplify.cpp explicitly?
> > > > Not handling which case in InstructionSimplify.cpp?
> > > > 
> > > > I can change the call, but I want to be wary of "separation of concerns". Normally, we just change the instruction and let other passes do the clean up. I don't want to put a whole lot of logic in here to DCE, simplify, etc., unless we know those passes won't run afterwards.
> > > Remember, in the -O0 pass chain SimplifyCFG is never run. That seems to be the only pass that changes conditional branches into unconditional branches. We always run DCE, so exploiting that one is fine. In D65280, one option of extending instruction simplify is implemented, but that one certainly needs some thoughts on whether it is the correct way forward.
> > Right. So I'm not sure about your comment that `replaceAndRecursivelySimplify` not being good enough matters, since -O0 has no guarantees about performance. The only way it could matter is if code that's expected to be dead, and would otherwise cause a compilation error of some sort, is left over.
> Exactly, the inline asm statements that triggered the original case. If they are left around, they can trigger the I-want-immediate-operands checks. 
God bless inline asm. :-)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64963