Skip to content

Commit 7b2fc48

Browse files
authored
[InstCombine] Remove dead poison check. NFCI (#141264)
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
1 parent 6520b21 commit 7b2fc48

File tree

2 files changed

+5
-21
lines changed

2 files changed

+5
-21
lines changed

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2254,25 +2254,6 @@ Instruction *InstCombinerImpl::foldVectorBinop(BinaryOperator &Inst) {
22542254
}
22552255
NewVecC[ShMask[I]] = CElt;
22562256
}
2257-
// If this is a widening shuffle, we must be able to extend with poison
2258-
// elements. If the original binop does not produce a poison in the high
2259-
// lanes, then this transform is not safe.
2260-
// Similarly for poison lanes due to the shuffle mask, we can only
2261-
// transform binops that preserve poison.
2262-
// TODO: We could shuffle those non-poison constant values into the
2263-
// result by using a constant vector (rather than an poison vector)
2264-
// as operand 1 of the new binop, but that might be too aggressive
2265-
// for target-independent shuffle creation.
2266-
if (I >= SrcVecNumElts || ShMask[I] < 0) {
2267-
Constant *MaybePoison =
2268-
ConstOp1
2269-
? ConstantFoldBinaryOpOperands(Opcode, PoisonScalar, CElt, DL)
2270-
: ConstantFoldBinaryOpOperands(Opcode, CElt, PoisonScalar, DL);
2271-
if (!MaybePoison || !isa<PoisonValue>(MaybePoison)) {
2272-
MayChange = false;
2273-
break;
2274-
}
2275-
}
22762257
}
22772258
if (MayChange) {
22782259
Constant *NewC = ConstantVector::get(NewVecC);

llvm/test/Transforms/InstCombine/vec_shuffle.ll

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -732,8 +732,11 @@ define <4 x i16> @widening_shuffle_shl_constant_op1_non0(<2 x i16> %v) {
732732
ret <4 x i16> %bo
733733
}
734734

735-
; A binop that does not produce undef in the high lanes can not be moved before the shuffle.
736-
; This is not ok because 'or -1, undef --> -1' but moving the shuffle results in undef instead.
735+
; Previously, a shufflevector would produce an undef element from an undef mask
736+
; index, which meant that pulling the shuffle out wasn't correct if the original
737+
; binary op produced a non-undef result, e.g. or -1, undef --> -1.
738+
;
739+
; However nowadays shufflevector produces poison, which is safe to propagate.
737740

738741
define <4 x i16> @widening_shuffle_or(<2 x i16> %v) {
739742
; CHECK-LABEL: @widening_shuffle_or(

0 commit comments

Comments
 (0)