[llvm] f2e93d1 - [CVP] prevent propagating poison when substituting edge values into a phi (PR43802)

[Sanjay Patel via llvm-commits]

Author: Sanjay Patel
Date: 2019-10-28T08:58:28-04:00
New Revision: f2e93d10fe0c7a845254d35f59f47d439e9ff89b

URL: https://github.com/llvm/llvm-project/commit/f2e93d10fe0c7a845254d35f59f47d439e9ff89b
DIFF: https://github.com/llvm/llvm-project/commit/f2e93d10fe0c7a845254d35f59f47d439e9ff89b.diff

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

This phi simplification transform was added with:
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).

Differential Revision: https://reviews.llvm.org/D69442

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
index 2ef85268df48..8c4402b1d04f 100644
--- a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
+++ b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
@@ -194,7 +194,14 @@ static bool simplifyCommonValuePhi(PHINode *P, LazyValueInfo *LVI,
   }
 
   // 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;

diff  --git a/llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll b/llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll
index 814dcba15865..d6afcc863cf6 100644
--- a/llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll
+++ b/llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll
@@ -123,14 +123,13 @@ return:
   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: