From 8e764f1ea32d5600479ba320f555b42531bf6cb8 Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Thu, 10 Oct 2024 20:09:09 +0100 Subject: [PATCH] InstCombine: extend select-equiv to support vectors foldSelectEquivalence currently doesn't support comparisons on vector types due to correctness concerns. Note that the only concern is lane-crossing; ShuffleVector is the only possible lane-crossing instruction, and ShuffleVectorInst::{isIdentity,isShuffle} are the exact properties that should not be broken for valid vector-replacements. Put in the checks, and lift the limitation. --- llvm/lib/Analysis/ValueTracking.cpp | 4 +++- llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp | 7 +++++-- llvm/test/Transforms/InstCombine/and-or-icmps.ll | 2 +- llvm/test/Transforms/InstCombine/select-binop-cmp.ll | 10 +++++----- .../Transforms/InstCombine/select-value-equivalence.ll | 6 +++--- 5 files changed, 17 insertions(+), 12 deletions(-) diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp index f83347e7cd2bba2..c71d17011d7a0d8 100644 --- a/llvm/lib/Analysis/ValueTracking.cpp +++ b/llvm/lib/Analysis/ValueTracking.cpp @@ -6950,7 +6950,9 @@ bool llvm::onlyUsedByLifetimeMarkersOrDroppableInsts(const Value *V) { bool llvm::isNotCrossLaneOperation(const Instruction *I) { if (auto *II = dyn_cast(I)) return isTriviallyVectorizable(II->getIntrinsicID()); - return !isa(I); + auto *Shuffle = dyn_cast(I); + return (!Shuffle || Shuffle->isSelect()) && + !isa(I); } bool llvm::isSafeToSpeculativelyExecute(const Instruction *Inst, diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp index 358563a5fcd537c..820d3608c8dc49c 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp @@ -1288,6 +1288,10 @@ bool InstCombinerImpl::replaceInInstruction(Value *V, Value *Old, Value *New, !isSafeToSpeculativelyExecuteWithVariableReplaced(I)) return false; + // Forbid potentially lane-crossing instructions. + if (Old->getType()->isVectorTy() && !isNotCrossLaneOperation(I)) + return false; + bool Changed = false; for (Use &U : I->operands()) { if (U == Old) { @@ -1366,9 +1370,8 @@ Instruction *InstCombinerImpl::foldSelectValueEquivalence(SelectInst &Sel, // with different operands, which should not cause side-effects or trigger // undefined behavior). Only do this if CmpRHS is a constant, as // profitability is not clear for other cases. - // FIXME: Support vectors. if (OldOp == CmpLHS && match(NewOp, m_ImmConstant()) && - !match(OldOp, m_Constant()) && !Cmp.getType()->isVectorTy() && + !match(OldOp, m_Constant()) && isGuaranteedNotToBeUndef(NewOp, SQ.AC, &Sel, &DT)) if (replaceInInstruction(TrueVal, OldOp, NewOp)) return &Sel; diff --git a/llvm/test/Transforms/InstCombine/and-or-icmps.ll b/llvm/test/Transforms/InstCombine/and-or-icmps.ll index ad28ad980de5b47..eb4723c86542de5 100644 --- a/llvm/test/Transforms/InstCombine/and-or-icmps.ll +++ b/llvm/test/Transforms/InstCombine/and-or-icmps.ll @@ -983,7 +983,7 @@ define <2 x i1> @substitute_constant_or_ne_slt_swap_vec_poison(<2 x i8> %x, <2 x define <2 x i1> @substitute_constant_or_ne_slt_swap_vec_logical(<2 x i8> %x, <2 x i8> %y) { ; CHECK-LABEL: @substitute_constant_or_ne_slt_swap_vec_logical( ; CHECK-NEXT: [[C1:%.*]] = icmp ne <2 x i8> [[X:%.*]], -; CHECK-NEXT: [[C2:%.*]] = icmp slt <2 x i8> [[Y:%.*]], [[X]] +; CHECK-NEXT: [[C2:%.*]] = icmp slt <2 x i8> [[Y:%.*]], ; CHECK-NEXT: [[R:%.*]] = select <2 x i1> [[C1]], <2 x i1> , <2 x i1> [[C2]] ; CHECK-NEXT: ret <2 x i1> [[R]] ; diff --git a/llvm/test/Transforms/InstCombine/select-binop-cmp.ll b/llvm/test/Transforms/InstCombine/select-binop-cmp.ll index 647287ef5ebad17..cd8c29ba4cd8196 100644 --- a/llvm/test/Transforms/InstCombine/select-binop-cmp.ll +++ b/llvm/test/Transforms/InstCombine/select-binop-cmp.ll @@ -552,12 +552,12 @@ define i32 @select_xor_icmp_bad_6(i32 %x, i32 %y, i32 %z) { ret i32 %C } -; Value equivalence substitution is all-or-nothing, so needs a scalar compare. +; Value equivalence substitution is valid. -define <2 x i8> @select_xor_icmp_vec_bad(<2 x i8> %x, <2 x i8> %y, <2 x i8> %z) { -; CHECK-LABEL: @select_xor_icmp_vec_bad( +define <2 x i8> @select_xor_icmp_vec_equivalence(<2 x i8> %x, <2 x i8> %y, <2 x i8> %z) { +; CHECK-LABEL: @select_xor_icmp_vec_equivalence( ; CHECK-NEXT: [[A:%.*]] = icmp eq <2 x i8> [[X:%.*]], -; CHECK-NEXT: [[B:%.*]] = xor <2 x i8> [[X]], [[Z:%.*]] +; CHECK-NEXT: [[B:%.*]] = xor <2 x i8> [[Z:%.*]], ; CHECK-NEXT: [[C:%.*]] = select <2 x i1> [[A]], <2 x i8> [[B]], <2 x i8> [[Y:%.*]] ; CHECK-NEXT: ret <2 x i8> [[C]] ; @@ -567,7 +567,7 @@ define <2 x i8> @select_xor_icmp_vec_bad(<2 x i8> %x, <2 x i8> %y, <2 x i8> %z) ret <2 x i8> %C } -; Value equivalence substitution is all-or-nothing, so needs a scalar compare. +; Value equivalence substitution is invalid due to lane-crossing shufflevector. define <2 x i32> @vec_select_no_equivalence(<2 x i32> %x) { ; CHECK-LABEL: @vec_select_no_equivalence( diff --git a/llvm/test/Transforms/InstCombine/select-value-equivalence.ll b/llvm/test/Transforms/InstCombine/select-value-equivalence.ll index 8d25458b17e27d1..28984282511924b 100644 --- a/llvm/test/Transforms/InstCombine/select-value-equivalence.ll +++ b/llvm/test/Transforms/InstCombine/select-value-equivalence.ll @@ -5,7 +5,7 @@ define <2 x i8> @select_icmp_insertelement_eq(<2 x i8> %x, <2 x i8> %y, i8 %i) { ; CHECK-LABEL: define <2 x i8> @select_icmp_insertelement_eq( ; CHECK-SAME: <2 x i8> [[X:%.*]], <2 x i8> [[Y:%.*]], i8 [[I:%.*]]) { ; CHECK-NEXT: [[CMP:%.*]] = icmp eq <2 x i8> [[Y]], -; CHECK-NEXT: [[INSERT:%.*]] = insertelement <2 x i8> [[Y]], i8 0, i8 [[I]] +; CHECK-NEXT: [[INSERT:%.*]] = insertelement <2 x i8> , i8 0, i8 [[I]] ; CHECK-NEXT: [[RETVAL:%.*]] = select <2 x i1> [[CMP]], <2 x i8> [[INSERT]], <2 x i8> [[X]] ; CHECK-NEXT: ret <2 x i8> [[RETVAL]] ; @@ -19,7 +19,7 @@ define <2 x i8> @select_icmp_insertelement_ne(<2 x i8> %x, <2 x i8> %y, i8 %i) { ; CHECK-LABEL: define <2 x i8> @select_icmp_insertelement_ne( ; CHECK-SAME: <2 x i8> [[X:%.*]], <2 x i8> [[Y:%.*]], i8 [[I:%.*]]) { ; CHECK-NEXT: [[CMP_NOT:%.*]] = icmp eq <2 x i8> [[Y]], -; CHECK-NEXT: [[INSERT:%.*]] = insertelement <2 x i8> [[Y]], i8 0, i8 [[I]] +; CHECK-NEXT: [[INSERT:%.*]] = insertelement <2 x i8> , i8 0, i8 [[I]] ; CHECK-NEXT: [[RETVAL:%.*]] = select <2 x i1> [[CMP_NOT]], <2 x i8> [[INSERT]], <2 x i8> [[X]] ; CHECK-NEXT: ret <2 x i8> [[RETVAL]] ; @@ -46,7 +46,7 @@ define <4 x i8> @select_icmp_shufflevector_select(<4 x i8> %x, <4 x i8> %y, <4 x ; CHECK-LABEL: define <4 x i8> @select_icmp_shufflevector_select( ; CHECK-SAME: <4 x i8> [[X:%.*]], <4 x i8> [[Y:%.*]], <4 x i8> [[Z:%.*]]) { ; CHECK-NEXT: [[CMP:%.*]] = icmp eq <4 x i8> [[Y]], -; CHECK-NEXT: [[SHUFFLE:%.*]] = shufflevector <4 x i8> [[Z]], <4 x i8> [[Y]], <4 x i32> +; CHECK-NEXT: [[SHUFFLE:%.*]] = shufflevector <4 x i8> [[Z]], <4 x i8> , <4 x i32> ; CHECK-NEXT: [[RETVAL:%.*]] = select <4 x i1> [[CMP]], <4 x i8> [[SHUFFLE]], <4 x i8> [[X]] ; CHECK-NEXT: ret <4 x i8> [[RETVAL]] ;