[InstCombine] Don't cover up poison elements for shifts when folding shuffles thru binops #141303
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…shuffles thru binops As noted in the TODO, we don't need to cover up the poison elements placed in the unused lanes for shifts, since it's not UB unlike div/rem. New poison elements are only introduced in cases like ShMask = <1,1,2,2> and C = <5,5,6,6> --> NewC = <poison,5,6,poison> And the resulting shuffle won't use the poison lanes.
llvmbot
added
llvm:instcombine
|
@llvm/pr-subscribers-llvm-transforms Author: Luke Lau (lukel97) ChangesAs noted in the TODO, we don't need to cover up the poison elements placed in the unused lanes for shifts, since it's not UB unlike div/rem. New poison elements are only introduced in cases like ShMask = <1,1,2,2> and C = <5,5,6,6> --> NewC = <poison,5,6,poison> And the resulting shuffle won't use the poison lanes. Full diff: https://github.com/llvm/llvm-project/pull/141303.diff 4 Files Affected:
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index c1608b1866a5d..46ed0913208cd 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2260,8 +2260,7 @@ Instruction *InstCombinerImpl::foldVectorBinop(BinaryOperator &Inst) {
// It may not be safe to execute a binop on a vector with poison elements
// because the entire instruction can be folded to undef or create poison
// that did not exist in the original code.
- // TODO: The shift case should not be necessary.
- if (Inst.isIntDivRem() || (Inst.isShift() && ConstOp1))
+ if (Inst.isIntDivRem())
NewC = getSafeVectorConstantForBinop(Opcode, NewC, ConstOp1);
// Op(shuffle(V1, Mask), C) -> shuffle(Op(V1, NewC), Mask)
diff --git a/llvm/test/Transforms/InstCombine/vec_shuffle-inseltpoison.ll b/llvm/test/Transforms/InstCombine/vec_shuffle-inseltpoison.ll
index 74d07fea9793a..9aa050e8cd500 100644
--- a/llvm/test/Transforms/InstCombine/vec_shuffle-inseltpoison.ll
+++ b/llvm/test/Transforms/InstCombine/vec_shuffle-inseltpoison.ll
@@ -880,7 +880,7 @@ define <2 x i32> @shl_splat_constant0(<2 x i32> %x) {
define <2 x i32> @shl_splat_constant1(<2 x i32> %x) {
; CHECK-LABEL: @shl_splat_constant1(
-; CHECK-NEXT: [[TMP1:%.*]] = shl <2 x i32> [[X:%.*]], <i32 5, i32 0>
+; CHECK-NEXT: [[TMP1:%.*]] = shl <2 x i32> [[X:%.*]], <i32 5, i32 poison>
; CHECK-NEXT: [[R:%.*]] = shufflevector <2 x i32> [[TMP1]], <2 x i32> poison, <2 x i32> zeroinitializer
; CHECK-NEXT: ret <2 x i32> [[R]]
;
@@ -902,7 +902,7 @@ define <2 x i32> @ashr_splat_constant0(<2 x i32> %x) {
define <2 x i32> @ashr_splat_constant1(<2 x i32> %x) {
; CHECK-LABEL: @ashr_splat_constant1(
-; CHECK-NEXT: [[TMP1:%.*]] = ashr <2 x i32> [[X:%.*]], <i32 5, i32 0>
+; CHECK-NEXT: [[TMP1:%.*]] = ashr <2 x i32> [[X:%.*]], <i32 5, i32 poison>
; CHECK-NEXT: [[R:%.*]] = shufflevector <2 x i32> [[TMP1]], <2 x i32> poison, <2 x i32> zeroinitializer
; CHECK-NEXT: ret <2 x i32> [[R]]
;
@@ -924,7 +924,7 @@ define <2 x i32> @lshr_splat_constant0(<2 x i32> %x) {
define <2 x i32> @lshr_splat_constant1(<2 x i32> %x) {
; CHECK-LABEL: @lshr_splat_constant1(
-; CHECK-NEXT: [[TMP1:%.*]] = lshr <2 x i32> [[X:%.*]], <i32 5, i32 0>
+; CHECK-NEXT: [[TMP1:%.*]] = lshr <2 x i32> [[X:%.*]], <i32 5, i32 poison>
; CHECK-NEXT: [[R:%.*]] = shufflevector <2 x i32> [[TMP1]], <2 x i32> poison, <2 x i32> zeroinitializer
; CHECK-NEXT: ret <2 x i32> [[R]]
;
@@ -1161,7 +1161,7 @@ entry:
define <4 x i16> @shl_constant_mask_undef(<4 x i16> %in) {
; CHECK-LABEL: @shl_constant_mask_undef(
; CHECK-NEXT: entry:
-; CHECK-NEXT: [[TMP0:%.*]] = shl <4 x i16> [[IN:%.*]], <i16 10, i16 0, i16 0, i16 0>
+; CHECK-NEXT: [[TMP0:%.*]] = shl <4 x i16> [[IN:%.*]], <i16 10, i16 0, i16 poison, i16 poison>
; CHECK-NEXT: [[SHL:%.*]] = shufflevector <4 x i16> [[TMP0]], <4 x i16> poison, <4 x i32> <i32 0, i32 poison, i32 1, i32 1>
; CHECK-NEXT: ret <4 x i16> [[SHL]]
;
diff --git a/llvm/test/Transforms/InstCombine/vec_shuffle.ll b/llvm/test/Transforms/InstCombine/vec_shuffle.ll
index eb88fbadab956..90ad88089054e 100644
--- a/llvm/test/Transforms/InstCombine/vec_shuffle.ll
+++ b/llvm/test/Transforms/InstCombine/vec_shuffle.ll
@@ -929,7 +929,7 @@ define <2 x i32> @shl_splat_constant0(<2 x i32> %x) {
define <2 x i32> @shl_splat_constant1(<2 x i32> %x) {
; CHECK-LABEL: @shl_splat_constant1(
-; CHECK-NEXT: [[TMP1:%.*]] = shl <2 x i32> [[X:%.*]], <i32 5, i32 0>
+; CHECK-NEXT: [[TMP1:%.*]] = shl <2 x i32> [[X:%.*]], <i32 5, i32 poison>
; CHECK-NEXT: [[R:%.*]] = shufflevector <2 x i32> [[TMP1]], <2 x i32> poison, <2 x i32> zeroinitializer
; CHECK-NEXT: ret <2 x i32> [[R]]
;
@@ -951,7 +951,7 @@ define <2 x i32> @ashr_splat_constant0(<2 x i32> %x) {
define <2 x i32> @ashr_splat_constant1(<2 x i32> %x) {
; CHECK-LABEL: @ashr_splat_constant1(
-; CHECK-NEXT: [[TMP1:%.*]] = ashr <2 x i32> [[X:%.*]], <i32 5, i32 0>
+; CHECK-NEXT: [[TMP1:%.*]] = ashr <2 x i32> [[X:%.*]], <i32 5, i32 poison>
; CHECK-NEXT: [[R:%.*]] = shufflevector <2 x i32> [[TMP1]], <2 x i32> poison, <2 x i32> zeroinitializer
; CHECK-NEXT: ret <2 x i32> [[R]]
;
@@ -973,7 +973,7 @@ define <2 x i32> @lshr_splat_constant0(<2 x i32> %x) {
define <2 x i32> @lshr_splat_constant1(<2 x i32> %x) {
; CHECK-LABEL: @lshr_splat_constant1(
-; CHECK-NEXT: [[TMP1:%.*]] = lshr <2 x i32> [[X:%.*]], <i32 5, i32 0>
+; CHECK-NEXT: [[TMP1:%.*]] = lshr <2 x i32> [[X:%.*]], <i32 5, i32 poison>
; CHECK-NEXT: [[R:%.*]] = shufflevector <2 x i32> [[TMP1]], <2 x i32> poison, <2 x i32> zeroinitializer
; CHECK-NEXT: ret <2 x i32> [[R]]
;
@@ -1210,7 +1210,7 @@ entry:
define <4 x i16> @shl_constant_mask_undef(<4 x i16> %in) {
; CHECK-LABEL: @shl_constant_mask_undef(
; CHECK-NEXT: entry:
-; CHECK-NEXT: [[TMP0:%.*]] = shl <4 x i16> [[IN:%.*]], <i16 10, i16 0, i16 0, i16 0>
+; CHECK-NEXT: [[TMP0:%.*]] = shl <4 x i16> [[IN:%.*]], <i16 10, i16 0, i16 poison, i16 poison>
; CHECK-NEXT: [[SHL:%.*]] = shufflevector <4 x i16> [[TMP0]], <4 x i16> poison, <4 x i32> <i32 0, i32 poison, i32 1, i32 1>
; CHECK-NEXT: ret <4 x i16> [[SHL]]
;
diff --git a/llvm/test/Transforms/LoopVectorize/AArch64/deterministic-type-shrinkage.ll b/llvm/test/Transforms/LoopVectorize/AArch64/deterministic-type-shrinkage.ll
index c2502aac5b61d..3e3d88e3cea2c 100644
--- a/llvm/test/Transforms/LoopVectorize/AArch64/deterministic-type-shrinkage.ll
+++ b/llvm/test/Transforms/LoopVectorize/AArch64/deterministic-type-shrinkage.ll
@@ -164,8 +164,8 @@ define void @test_shrink_zext_in_preheader(ptr noalias %src, ptr noalias %dst, i
; CHECK-NEXT: [[TMP1:%.*]] = insertelement <16 x i16> poison, i16 [[TMP0]], i64 0
; CHECK-NEXT: [[TMP2:%.*]] = insertelement <16 x i16> poison, i16 [[B]], i64 0
; CHECK-NEXT: [[TMP3:%.*]] = mul <16 x i16> [[TMP1]], [[TMP2]]
-; CHECK-NEXT: [[TMP4:%.*]] = lshr <16 x i16> [[TMP3]], <i16 8, i16 0, i16 0, i16 0, i16 0, i16 0, i16 0, i16 0, i16 0, i16 0, i16 0, i16 0, i16 0, i16 0, i16 0, i16 0>
-; CHECK-NEXT: [[TMP5:%.*]] = trunc <16 x i16> [[TMP4]] to <16 x i8>
+; CHECK-NEXT: [[TMP4:%.*]] = lshr <16 x i16> [[TMP3]], <i16 8, i16 poison, i16 poison, i16 poison, i16 poison, i16 poison, i16 poison, i16 poison, i16 poison, i16 poison, i16 poison, i16 poison, i16 poison, i16 poison, i16 poison, i16 poison>
+; CHECK-NEXT: [[TMP5:%.*]] = trunc nuw <16 x i16> [[TMP4]] to <16 x i8>
; CHECK-NEXT: [[TMP6:%.*]] = shufflevector <16 x i8> [[TMP5]], <16 x i8> poison, <16 x i32> zeroinitializer
; CHECK-NEXT: br label [[VECTOR_BODY:%.*]]
; CHECK: vector.body:
@@ -187,8 +187,8 @@ define void @test_shrink_zext_in_preheader(ptr noalias %src, ptr noalias %dst, i
; CHECK-NEXT: [[TMP12:%.*]] = insertelement <8 x i16> poison, i16 [[TMP11]], i64 0
; CHECK-NEXT: [[TMP13:%.*]] = insertelement <8 x i16> poison, i16 [[B]], i64 0
; CHECK-NEXT: [[TMP14:%.*]] = mul <8 x i16> [[TMP12]], [[TMP13]]
-; CHECK-NEXT: [[TMP15:%.*]] = lshr <8 x i16> [[TMP14]], <i16 8, i16 0, i16 0, i16 0, i16 0, i16 0, i16 0, i16 0>
-; CHECK-NEXT: [[TMP16:%.*]] = trunc <8 x i16> [[TMP15]] to <8 x i8>
+; CHECK-NEXT: [[TMP15:%.*]] = lshr <8 x i16> [[TMP14]], <i16 8, i16 poison, i16 poison, i16 poison, i16 poison, i16 poison, i16 poison, i16 poison>
+; CHECK-NEXT: [[TMP16:%.*]] = trunc nuw <8 x i16> [[TMP15]] to <8 x i8>
; CHECK-NEXT: [[TMP17:%.*]] = shufflevector <8 x i8> [[TMP16]], <8 x i8> poison, <8 x i32> zeroinitializer
; CHECK-NEXT: br label [[VEC_EPILOG_VECTOR_BODY:%.*]]
; CHECK: vec.epilog.vector.body:
|
dtcxzyw
approved these changes
May 24, 2025
| @@ -2260,8 +2260,7 @@ Instruction *InstCombinerImpl::foldVectorBinop(BinaryOperator &Inst) { | |||
| // It may not be safe to execute a binop on a vector with poison elements | |||
| // because the entire instruction can be folded to undef or create poison | |||
| // that did not exist in the original code. | |||
| // TODO: The shift case should not be necessary. | |||
| if (Inst.isIntDivRem() || (Inst.isShift() && ConstOp1)) | |||
| if (Inst.isIntDivRem()) | |||
There was a problem hiding this comment.
The comment above is outdated.
See also 7cd3241
sivan-shani
pushed a commit
to sivan-shani/llvm-project
that referenced
this pull request
Jun 3, 2025
…shuffles thru binops (llvm#141303) As noted in the TODO, we don't need to cover up the poison elements placed in the unused lanes for shifts, since it's not UB unlike div/rem. New poison elements are only introduced in cases like ShMask = <1,1,2,2> and C = <5,5,6,6> --> NewC = <poison,5,6,poison> And the resulting shuffle won't use the poison lanes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As noted in the TODO, we don't need to cover up the poison elements placed in the unused lanes for shifts, since it's not UB unlike div/rem.
New poison elements are only introduced in cases like
ShMask = <1,1,2,2> and C = <5,5,6,6> --> NewC = <poison,5,6,poison>
And the resulting shuffle won't use the poison lanes.