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

[Sanjay Patel via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf2e93d10fe0c: [CVP] prevent propagating poison when substituting edge values into a phi… (authored by spatel).

Changed prior to commit:
  https://reviews.llvm.org/D69442?vs=226478&id=226650#toc

Repository:
  rG LLVM Github Monorepo

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

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,14 @@
   }
 
   // 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.
+  // Warning: If the underlying analysis changes, this may not be enough to
+  //          guarantee that poison is not propagated.
+  // TODO: We may be able to re-infer flags by re-analyzing the instruction.
+  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.226650.patch
Type: text/x-patch
Size: 1839 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191028/ffcb0ee2/attachment.bin>