From 4ca25c66d41ab136d1587b33571d3e58cd5989f8 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Mon, 12 Sep 2022 12:03:21 -0400 Subject: [PATCH] [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. --- llvm/lib/Transforms/Scalar/Reassociate.cpp | 6 ++++++ llvm/test/Transforms/Reassociate/negation.ll | 8 +++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/Reassociate.cpp b/llvm/lib/Transforms/Scalar/Reassociate.cpp index 1b11ec6..21628b6 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(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 d66cf87..21b2112 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> , [[X:%.*]] ; CHECK-NEXT: [[SHUF:%.*]] = shufflevector <2 x i32> [[PARTIAL_NEG]], <2 x i32> [[X]], <2 x i32> -; CHECK-NEXT: [[SUB:%.*]] = add <2 x i32> [[PARTIAL_NEG]], +; CHECK-NEXT: [[X_NEG:%.*]] = sub <2 x i32> zeroinitializer, [[X]] +; CHECK-NEXT: [[SUB:%.*]] = add <2 x i32> [[X_NEG]], ; 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> , [[X:%.*]] ; CHECK-NEXT: [[SHUF:%.*]] = shufflevector <2 x float> [[PARTIAL_NEG]], <2 x float> [[X]], <2 x i32> -; CHECK-NEXT: [[SUB:%.*]] = fadd reassoc nsz <2 x float> [[PARTIAL_NEG]], +; CHECK-NEXT: [[X_NEG:%.*]] = fneg reassoc nsz <2 x float> [[X]] +; CHECK-NEXT: [[SUB:%.*]] = fadd reassoc nsz <2 x float> [[X_NEG]], ; CHECK-NEXT: [[R:%.*]] = fadd reassoc nsz <2 x float> [[SUB]], [[SHUF]] ; CHECK-NEXT: ret <2 x float> [[R]] ; -- 2.7.4