[InstCombine] Remove dead poison check. NFCI

[lukel97]

As far as I understand any binary op with poison as either operand will constant fold to poison, so this check will never trigger. llvm::ConstantFoldBinaryInstruction seems to confirm this?

I think this ended up getting left behind because originally shufflevectors with undef indices produced undef elements, and we couldn't pull the shuffle across some binops like or undef, -1 --> -1.

This code was added in 8c65515 to partially fix it and further extended in f749901, originally checking for undef but changed to check for poison in cd54c47

But nowadays shufflevectors with undef indices are treated as poison indices as of 575fdea, and so produce poison elements, so this is no longer an issue

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Luke Lau (lukel97)

Changes

As far as I understand any binary op with poison as either operand will constant fold to poison, so this check will never trigger. llvm::ConstantFoldBinaryInstruction seems to confirm this?

I think this ended up getting left behind because originally shufflevectors with undef indices produced undef elements, and we couldn't pull the shuffle across some binops like or undef, -1 --> -1.

This code was added in 8c65515 to partially fix it and further extended in f749901.

But nowadays shufflevectors with undef indices are treated as poison indices, and so produce poison elements, so this is no longer an issue

---

Full diff: https://github.com/llvm/llvm-project/pull/141264.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/InstCombine/InstructionCombining.cpp (-19)
• (modified) llvm/test/Transforms/InstCombine/vec_shuffle.ll (+5-2)

diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 24026e310ad11..c1608b1866a5d 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2254,25 +2254,6 @@ Instruction *InstCombinerImpl::foldVectorBinop(BinaryOperator &Inst) {
         }
         NewVecC[ShMask[I]] = CElt;
       }
-      // If this is a widening shuffle, we must be able to extend with poison
-      // elements. If the original binop does not produce a poison in the high
-      // lanes, then this transform is not safe.
-      // Similarly for poison lanes due to the shuffle mask, we can only
-      // transform binops that preserve poison.
-      // TODO: We could shuffle those non-poison constant values into the
-      //       result by using a constant vector (rather than an poison vector)
-      //       as operand 1 of the new binop, but that might be too aggressive
-      //       for target-independent shuffle creation.
-      if (I >= SrcVecNumElts || ShMask[I] < 0) {
-        Constant *MaybePoison =
-            ConstOp1
-                ? ConstantFoldBinaryOpOperands(Opcode, PoisonScalar, CElt, DL)
-                : ConstantFoldBinaryOpOperands(Opcode, CElt, PoisonScalar, DL);
-        if (!MaybePoison || !isa<PoisonValue>(MaybePoison)) {
-          MayChange = false;
-          break;
-        }
-      }
     }
     if (MayChange) {
       Constant *NewC = ConstantVector::get(NewVecC);
diff --git a/llvm/test/Transforms/InstCombine/vec_shuffle.ll b/llvm/test/Transforms/InstCombine/vec_shuffle.ll
index 53f9df91d6af8..eb88fbadab956 100644
--- a/llvm/test/Transforms/InstCombine/vec_shuffle.ll
+++ b/llvm/test/Transforms/InstCombine/vec_shuffle.ll
@@ -732,8 +732,11 @@ define <4 x i16> @widening_shuffle_shl_constant_op1_non0(<2 x i16> %v) {
   ret <4 x i16> %bo
 }
 
-; A binop that does not produce undef in the high lanes can not be moved before the shuffle.
-; This is not ok because 'or -1, undef --> -1' but moving the shuffle results in undef instead.
+; Previously, a shufflevector would produce an undef element from an undef mask
+; index, which meant that pulling the shuffle out wasn't correct if the original
+; binary op produced a non-undef result, e.g. or -1, undef --> -1.
+;
+; However nowadays shufflevector produces poison, which is safe to propagate.
 
 define <4 x i16> @widening_shuffle_or(<2 x i16> %v) {
 ; CHECK-LABEL: @widening_shuffle_or(

[nikic]

LGTM