[llvm] a8f1a3e - [SLP]Fix PR63141: compareCmp is not strict weak ordering.
Alexey Bataev via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 28 06:06:30 PDT 2023
Author: Alexey Bataev
Date: 2023-06-28T06:00:31-07:00
New Revision: a8f1a3e025c905f015a622809e58b03f09b7a58e
URL: https://github.com/llvm/llvm-project/commit/a8f1a3e025c905f015a622809e58b03f09b7a58e
DIFF: https://github.com/llvm/llvm-project/commit/a8f1a3e025c905f015a622809e58b03f09b7a58e.diff
LOG: [SLP]Fix PR63141: compareCmp is not strict weak ordering.
Added some extra checks for comapreCMP function if IsCompatibility is
false to make it meat the strict weak ordering requirements to be
correctly used in sort functions.
Added:
Modified:
llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
llvm/test/Transforms/SLPVectorizer/X86/alternate-cmp-swapped-pred-parent.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 3416e953e099bb..f76e49274c5905 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -14413,11 +14413,12 @@ static bool tryToVectorizeSequence(
/// of the second cmp instruction.
template <bool IsCompatibility>
static bool compareCmp(Value *V, Value *V2, TargetLibraryInfo &TLI,
- function_ref<bool(Instruction *)> IsDeleted) {
+ const DominatorTree &DT) {
+ assert(isValidElementType(V->getType()) &&
+ isValidElementType(V2->getType()) &&
+ "Expected valid element types only.");
auto *CI1 = cast<CmpInst>(V);
auto *CI2 = cast<CmpInst>(V2);
- if (IsDeleted(CI2) || !isValidElementType(CI2->getType()))
- return false;
if (CI1->getOperand(0)->getType()->getTypeID() <
CI2->getOperand(0)->getType()->getTypeID())
return !IsCompatibility;
@@ -14446,12 +14447,27 @@ static bool compareCmp(Value *V, Value *V2, TargetLibraryInfo &TLI,
return false;
if (auto *I1 = dyn_cast<Instruction>(Op1))
if (auto *I2 = dyn_cast<Instruction>(Op2)) {
- if (I1->getParent() != I2->getParent())
- return false;
+ if (IsCompatibility) {
+ if (I1->getParent() != I2->getParent())
+ return false;
+ } else {
+ // Try to compare nodes with same parent.
+ DomTreeNodeBase<BasicBlock> *NodeI1 = DT.getNode(I1->getParent());
+ DomTreeNodeBase<BasicBlock> *NodeI2 = DT.getNode(I2->getParent());
+ if (!NodeI1)
+ return NodeI2 != nullptr;
+ if (!NodeI2)
+ return false;
+ assert((NodeI1 == NodeI2) ==
+ (NodeI1->getDFSNumIn() == NodeI2->getDFSNumIn()) &&
+ "Different nodes should have
diff erent DFS numbers");
+ if (NodeI1 != NodeI2)
+ return NodeI1->getDFSNumIn() < NodeI2->getDFSNumIn();
+ }
InstructionsState S = getSameOpcode({I1, I2}, TLI);
- if (S.getOpcode())
+ if (S.getOpcode() && (IsCompatibility || !S.isAltShuffle()))
continue;
- return false;
+ return !IsCompatibility && I1->getOpcode() < I2->getOpcode();
}
}
return IsCompatibility;
@@ -14478,18 +14494,23 @@ bool SLPVectorizerPass::vectorizeCmpInsts(iterator_range<ItT> CmpInsts,
// Try to vectorize list of compares.
// Sort by type, compare predicate, etc.
auto CompareSorter = [&](Value *V, Value *V2) {
- return compareCmp<false>(V, V2, *TLI,
- [&R](Instruction *I) { return R.isDeleted(I); });
+ if (V == V2)
+ return false;
+ return compareCmp<false>(V, V2, *TLI, *DT);
};
auto AreCompatibleCompares = [&](Value *V1, Value *V2) {
if (V1 == V2)
return true;
- return compareCmp<true>(V1, V2, *TLI,
- [&R](Instruction *I) { return R.isDeleted(I); });
+ return compareCmp<true>(V1, V2, *TLI, *DT);
};
- SmallVector<Value *> Vals(CmpInsts.begin(), CmpInsts.end());
+ SmallVector<Value *> Vals;
+ for (Instruction *V : CmpInsts)
+ if (!R.isDeleted(V) && isValidElementType(V->getType()))
+ Vals.push_back(V);
+ if (Vals.size() <= 1)
+ return Changed;
Changed |= tryToVectorizeSequence<Value>(
Vals, CompareSorter, AreCompatibleCompares,
[this, &R](ArrayRef<Value *> Candidates, bool MaxVFOnly) {
diff --git a/llvm/test/Transforms/SLPVectorizer/X86/alternate-cmp-swapped-pred-parent.ll b/llvm/test/Transforms/SLPVectorizer/X86/alternate-cmp-swapped-pred-parent.ll
index 1e11a01f9774f5..a2a79bf53da250 100644
--- a/llvm/test/Transforms/SLPVectorizer/X86/alternate-cmp-swapped-pred-parent.ll
+++ b/llvm/test/Transforms/SLPVectorizer/X86/alternate-cmp-swapped-pred-parent.ll
@@ -39,10 +39,10 @@ define void @test1() {
; CHECK-NEXT: br label [[BB:%.*]]
; CHECK: bb:
; CHECK-NEXT: [[CALL37:%.*]] = load i16, ptr poison, align 2
-; CHECK-NEXT: [[TMP0:%.*]] = insertelement <8 x i16> <i16 0, i16 0, i16 0, i16 poison, i16 poison, i16 0, i16 poison, i16 poison>, i16 [[CALL37]], i32 3
-; CHECK-NEXT: [[TMP1:%.*]] = insertelement <8 x i16> [[TMP0]], i16 [[CALL]], i32 7
-; CHECK-NEXT: [[TMP2:%.*]] = shufflevector <8 x i16> [[TMP1]], <8 x i16> poison, <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 3, i32 5, i32 3, i32 7>
-; CHECK-NEXT: [[TMP3:%.*]] = shufflevector <8 x i16> [[TMP1]], <8 x i16> <i16 0, i16 0, i16 0, i16 0, i16 0, i16 poison, i16 poison, i16 0>, <8 x i32> <i32 8, i32 9, i32 10, i32 11, i32 12, i32 3, i32 7, i32 15>
+; CHECK-NEXT: [[TMP0:%.*]] = insertelement <8 x i16> <i16 0, i16 0, i16 0, i16 poison, i16 poison, i16 poison, i16 poison, i16 0>, i16 [[CALL]], i32 3
+; CHECK-NEXT: [[TMP1:%.*]] = insertelement <8 x i16> [[TMP0]], i16 [[CALL37]], i32 4
+; CHECK-NEXT: [[TMP2:%.*]] = shufflevector <8 x i16> [[TMP1]], <8 x i16> poison, <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 4, i32 4, i32 7>
+; CHECK-NEXT: [[TMP3:%.*]] = shufflevector <8 x i16> [[TMP1]], <8 x i16> <i16 0, i16 0, i16 0, i16 0, i16 0, i16 0, i16 poison, i16 poison>, <8 x i32> <i32 8, i32 9, i32 10, i32 11, i32 12, i32 13, i32 3, i32 4>
; CHECK-NEXT: [[TMP4:%.*]] = icmp slt <8 x i16> [[TMP2]], [[TMP3]]
; CHECK-NEXT: ret void
;
More information about the llvm-commits
mailing list