[PATCH] D69442: [CVP] prevent propagating poison when substituting edge values into a phi (PR43802)

[Sanjay Patel via Phabricator via llvm-commits]

spatel created this revision.
spatel added reviewers: DaniilSuchkov, fhahn, lebedev.ri, nlopes, nikic.
Herald added subscribers: hiraditya, mcrosier.
Herald added a project: LLVM.

This phi simplification transform was added with:
D45448 <https://reviews.llvm.org/D45448>

However as shown in PR43802:
https://bugs.llvm.org/show_bug.cgi?id=43802

...we must be careful not to propagate poison when we do the substitution. There might be some more complicated analysis possible to retain the overflow flag, but it should always be safe and easy to drop flags (we have similar behavior in instcombine and other passes).


https://reviews.llvm.org/D69442

Files:
  llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
  llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll


Index: llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll
===================================================================
--- llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll
+++ llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll
@@ -123,14 +123,13 @@
   ret i8* %r
 }
 
-; FIXME:
 ; The sub has 'nsw', so it is not safe to propagate that value along
 ; the bb2 edge because that would propagate poison to the return.
 
 define i32 @PR43802(i32 %arg) {
 ; CHECK-LABEL: @PR43802(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[SUB:%.*]] = sub nsw i32 0, [[ARG:%.*]]
+; CHECK-NEXT:    [[SUB:%.*]] = sub i32 0, [[ARG:%.*]]
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[ARG]], -2147483648
 ; CHECK-NEXT:    br i1 [[CMP]], label [[BB2:%.*]], label [[BB3:%.*]]
 ; CHECK:       bb2:
Index: llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
+++ llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
@@ -194,7 +194,11 @@
   }
 
   // All constant incoming values map to the same variable along the incoming
-  // edges of the phi. The phi is unnecessary.
+  // edges of the phi. The phi is unnecessary. However, we must drop all
+  // poison-generating flags to ensure that no poison is propagated to the phi
+  // location by performing this substitution.
+  if (auto *CommonInst = dyn_cast<Instruction>(CommonValue))
+    CommonInst->dropPoisonGeneratingFlags();
   P->replaceAllUsesWith(CommonValue);
   P->eraseFromParent();
   ++NumPhiCommon;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D69442.226478.patch
Type: text/x-patch
Size: 1628 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191025/19ab78e4/attachment.bin>