[PATCH] D36592: [BDCE] clear poison generators after turning a value into zero (PR33695, PR34037)

Hal Finkel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Aug 12 07:37:53 PDT 2017


hfinkel added a comment.

The comments about assume, otherwise, LGTM too.



================
Comment at: lib/Transforms/Scalar/BDCE.cpp:71
+    auto *II = dyn_cast<IntrinsicInst>(J);
+    if (II && II->getIntrinsicID() == Intrinsic::assume)
+      II->eraseFromParent();
----------------
I've changed my mind about assume. I believe, now that you're checking the demanded bits, you can't hit this case. The issue is that the assume demands its operand, and it must do this. Essentially, the value of the condition is an observable of the program (i.e. trivializing values can't change it). If there's a compare feeding the assume, for example, it in turn demands whatever bits are necessary in order to ensure its result. As a result, the compare shouldn't imply anything about its operands that trivializing changes (if it did, then the truth of the condition would not have have universally implied that thing in the first place). Thus, I think you can just remove the assume handling.


================
Comment at: lib/Transforms/Scalar/BDCE.cpp:72
+    if (II && II->getIntrinsicID() == Intrinsic::assume)
+      II->eraseFromParent();
+
----------------
You need a continue after this. You can't then dereference J.


https://reviews.llvm.org/D36592





More information about the llvm-commits mailing list