[llvm] 4ca25c6 - [Reassociate] prevent partial undef negation replacement

[Sanjay Patel via llvm-commits]

Author: Sanjay Patel
Date: 2022-09-12T12:28:34-04:00
New Revision: 4ca25c66d41ab136d1587b33571d3e58cd5989f8

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

LOG: [Reassociate] prevent partial undef negation replacement

As shown in the examples in issue #57683, we allow matching
vectors with poison (undef) in this transform (and possibly more),
but we can't then use the partially defined value as a replacement
value in other expressions blindly.

This seems to be avoided in simpler examples of reassociation,
and other passes should be able to clean up the redundant op
seen in these tests.

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/Reassociate.cpp
    llvm/test/Transforms/Reassociate/negation.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/Reassociate.cpp b/llvm/lib/Transforms/Scalar/Reassociate.cpp
index 1b11ec67f2320..21628b61edd62 100644
--- a/llvm/lib/Transforms/Scalar/Reassociate.cpp
+++ b/llvm/lib/Transforms/Scalar/Reassociate.cpp
@@ -887,6 +887,12 @@ static Value *NegateValue(Value *V, Instruction *BI,
     // be zapped by reassociate later, so we don't need much finesse here.
     Instruction *TheNeg = dyn_cast<Instruction>(U);
 
+    // We can't safely propagate a vector zero constant with poison/undef lanes.
+    Constant *C;
+    if (match(TheNeg, m_BinOp(m_Constant(C), m_Value())) &&
+        C->containsUndefOrPoisonElement())
+      continue;
+
     // Verify that the negate is in this function, V might be a constant expr.
     if (!TheNeg ||
         TheNeg->getParent()->getParent() != BI->getParent()->getParent())

diff  --git a/llvm/test/Transforms/Reassociate/negation.ll b/llvm/test/Transforms/Reassociate/negation.ll
index d66cf87f1c7f2..21b21122e282a 100644
--- a/llvm/test/Transforms/Reassociate/negation.ll
+++ b/llvm/test/Transforms/Reassociate/negation.ll
@@ -44,13 +44,14 @@ define <2 x i32> @negate_vec_undefs(<2 x i32> %a, <2 x i32> %b, <2 x i32> %z) {
   ret <2 x i32> %f
 }
 
-; FIXME: Replacing x with a partial undef negation is a miscompile.
+; Replacing %x with a partial undef negation is a miscompile.
 
 define <2 x i32> @PR57683(<2 x i32> %x) {
 ; CHECK-LABEL: @PR57683(
 ; CHECK-NEXT:    [[PARTIAL_NEG:%.*]] = sub <2 x i32> <i32 poison, i32 0>, [[X:%.*]]
 ; CHECK-NEXT:    [[SHUF:%.*]] = shufflevector <2 x i32> [[PARTIAL_NEG]], <2 x i32> [[X]], <2 x i32> <i32 1, i32 3>
-; CHECK-NEXT:    [[SUB:%.*]] = add <2 x i32> [[PARTIAL_NEG]], <i32 1, i32 1>
+; CHECK-NEXT:    [[X_NEG:%.*]] = sub <2 x i32> zeroinitializer, [[X]]
+; CHECK-NEXT:    [[SUB:%.*]] = add <2 x i32> [[X_NEG]], <i32 1, i32 1>
 ; CHECK-NEXT:    [[R:%.*]] = add <2 x i32> [[SUB]], [[SHUF]]
 ; CHECK-NEXT:    ret <2 x i32> [[R]]
 ;
@@ -65,7 +66,8 @@ define <2 x float> @PR57683_FP(<2 x float> %x) {
 ; CHECK-LABEL: @PR57683_FP(
 ; CHECK-NEXT:    [[PARTIAL_NEG:%.*]] = fsub reassoc nsz <2 x float> <float poison, float 0.000000e+00>, [[X:%.*]]
 ; CHECK-NEXT:    [[SHUF:%.*]] = shufflevector <2 x float> [[PARTIAL_NEG]], <2 x float> [[X]], <2 x i32> <i32 1, i32 3>
-; CHECK-NEXT:    [[SUB:%.*]] = fadd reassoc nsz <2 x float> [[PARTIAL_NEG]], <float 1.000000e+00, float 1.000000e+00>
+; CHECK-NEXT:    [[X_NEG:%.*]] = fneg reassoc nsz <2 x float> [[X]]
+; CHECK-NEXT:    [[SUB:%.*]] = fadd reassoc nsz <2 x float> [[X_NEG]], <float 1.000000e+00, float 1.000000e+00>
 ; CHECK-NEXT:    [[R:%.*]] = fadd reassoc nsz <2 x float> [[SUB]], [[SHUF]]
 ; CHECK-NEXT:    ret <2 x float> [[R]]
 ;