[InstCombine] Fix miscompile in negation of select

Swapping the operands of a select is not valid if one hand is
more poisonous that the other, because the negation zero contains
poison elements.

Fix this by adding an extra parameter to isKnownNegation() to
forbid poison elements.

I've implemented this using manual checks to avoid needing four
variants for the NeedsNSW/AllowPoison combinations. Maybe there is
a better way to do this...

Fixes #89669.

Author: nikic

llvm/include/llvm/Analysis/ValueTracking.h

@@ -130,7 +130,8 @@ bool isKnownNonZero(const Value *V, const SimplifyQuery &Q, unsigned Depth = 0);
 /// Currently can recoginze Value pair:
 /// 1: <X, Y> if X = sub (0, Y) or Y = sub (0, X)
 /// 2: <X, Y> if X = sub (A, B) and Y = sub (B, A)
-bool isKnownNegation(const Value *X, const Value *Y, bool NeedNSW = false);
+bool isKnownNegation(const Value *X, const Value *Y, bool NeedNSW = false,
+                     bool AllowPoison = true);
 
 /// Returns true if the give value is known to be non-negative.
 bool isKnownNonNegative(const Value *V, const SimplifyQuery &SQ,

llvm/lib/Analysis/ValueTracking.cpp

@@ -8042,17 +8042,28 @@ static SelectPatternResult matchMinMax(CmpInst::Predicate Pred,
   return {SPF_UNKNOWN, SPNB_NA, false};
 }
 
-bool llvm::isKnownNegation(const Value *X, const Value *Y, bool NeedNSW) {
+bool llvm::isKnownNegation(const Value *X, const Value *Y, bool NeedNSW,
+                           bool AllowPoison) {
   assert(X && Y && "Invalid operand");
 
-  // X = sub (0, Y) || X = sub nsw (0, Y)
-  if ((!NeedNSW && match(X, m_Sub(m_ZeroInt(), m_Specific(Y)))) ||
-      (NeedNSW && match(X, m_NSWNeg(m_Specific(Y)))))
+  auto IsNegationOf = [&](const Value *X, const Value *Y) {
+    if (!match(X, m_Neg(m_Specific(Y))))
+      return false;
+
+    auto *BO = cast<BinaryOperator>(X);
+    if (NeedNSW && !BO->hasNoSignedWrap())
+      return false;
+
+    auto *Zero = cast<Constant>(BO->getOperand(0));
+    if (!AllowPoison && !Zero->isNullValue())
+      return false;
+
     return true;
+  };
 
+  // X = sub (0, Y) || X = sub nsw (0, Y)
   // Y = sub (0, X) || Y = sub nsw (0, X)
-  if ((!NeedNSW && match(Y, m_Sub(m_ZeroInt(), m_Specific(X)))) ||
-      (NeedNSW && match(Y, m_NSWNeg(m_Specific(X)))))
+  if (IsNegationOf(X, Y) || IsNegationOf(Y, X))
     return true;
 
   // X = sub (A, B), Y = sub (B, A) || X = sub nsw (A, B), Y = sub nsw (B, A)

llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp

@@ -320,7 +320,8 @@ std::array<Value *, 2> Negator::getSortedOperandsOfBinOp(Instruction *I) {
     return NegatedPHI;
   }
   case Instruction::Select: {
-    if (isKnownNegation(I->getOperand(1), I->getOperand(2))) {
+    if (isKnownNegation(I->getOperand(1), I->getOperand(2), /*NeedNSW=*/false,
+                        /*AllowPoison=*/false)) {
       // Of one hand of select is known to be negation of another hand,
       // just swap the hands around.
       auto *NewSelect = cast<SelectInst>(I->clone());

llvm/test/Transforms/InstCombine/sub-of-negatible.ll

@@ -1385,12 +1385,12 @@ define i8 @dont_negate_ordinary_select(i8 %x, i8 %y, i8 %z, i1 %c) {
   ret i8 %t1
 }
 
-; FIXME: This is a miscompile.
 define <2 x i32> @negate_select_of_negation_poison(<2 x i1> %c, <2 x i32> %x) {
 ; CHECK-LABEL: @negate_select_of_negation_poison(
 ; CHECK-NEXT:    [[NEG:%.*]] = sub <2 x i32> <i32 0, i32 poison>, [[X:%.*]]
-; CHECK-NEXT:    [[TMP1:%.*]] = select <2 x i1> [[C:%.*]], <2 x i32> [[X]], <2 x i32> [[NEG]]
-; CHECK-NEXT:    ret <2 x i32> [[TMP1]]
+; CHECK-NEXT:    [[SEL:%.*]] = select <2 x i1> [[C:%.*]], <2 x i32> [[NEG]], <2 x i32> [[X]]
+; CHECK-NEXT:    [[NEG2:%.*]] = sub <2 x i32> zeroinitializer, [[SEL]]
+; CHECK-NEXT:    ret <2 x i32> [[NEG2]]
 ;
   %neg = sub <2 x i32> <i32 0, i32 poison>, %x
   %sel = select <2 x i1> %c, <2 x i32> %neg, <2 x i32> %x