[SLP]Support revectorization of the previously vectorized scalars #133091
Conversation
This file contains 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
Created using spr 1.3.5
|
@llvm/pr-subscribers-vectorizers @llvm/pr-subscribers-llvm-transforms Author: Alexey Bataev (alexey-bataev) ChangesIf the scalar instructions is marked for the vectorization in the tree, Patch is 34.05 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/133091.diff 8 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 59a0408abbf04..af3f61b98590d 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -4078,11 +4078,6 @@ class BoUpSLP {
if (isa<PoisonValue>(V))
continue;
auto It = ScalarToTreeEntries.find(V);
- assert(
- (It == ScalarToTreeEntries.end() ||
- (It->getSecond().size() == 1 && It->getSecond().front() == Last) ||
- doesNotNeedToBeScheduled(V)) &&
- "Scalar already in tree!");
if (It == ScalarToTreeEntries.end()) {
ScalarToTreeEntries.try_emplace(V).first->getSecond().push_back(Last);
(void)Processed.insert(V);
@@ -4342,6 +4337,9 @@ class BoUpSLP {
private:
/// Used for getting a "good" final ordering of instructions.
int SchedulingPriority = 0;
+ /// True if this instruction (or bundle) is scheduled (or considered as
+ /// scheduled in the dry-run).
+ bool IsScheduled = false;
/// The kind of the ScheduleEntity.
const Kind K = Kind::ScheduleData;
@@ -4355,6 +4353,10 @@ class BoUpSLP {
return SD->isReady();
return cast<ScheduleBundle>(this)->isReady();
}
+ /// Gets/sets if the bundle is scheduled.
+ bool isScheduled() const { return IsScheduled; }
+ void setScheduled(bool Scheduled) { IsScheduled = Scheduled; }
+
static bool classof(const ScheduleEntity *) { return true; }
};
@@ -4427,10 +4429,6 @@ class BoUpSLP {
IsScheduled = false;
}
- /// Gets/sets if the bundle is scheduled.
- bool isScheduled() const { return IsScheduled; }
- void setScheduled(bool Scheduled) { IsScheduled = Scheduled; }
-
/// Gets the number of unscheduled dependencies.
int getUnscheduledDeps() const { return UnscheduledDeps; }
/// Gets the number of dependencies.
@@ -4505,10 +4503,6 @@ class BoUpSLP {
/// for scheduling.
/// Note that this is negative as long as Dependencies is not calculated.
int UnscheduledDeps = InvalidDeps;
-
- /// True if this instruction is scheduled (or considered as scheduled in the
- /// dry-run).
- bool IsScheduled = false;
};
#ifndef NDEBUG
@@ -4553,11 +4547,6 @@ class BoUpSLP {
}
}
- bool isScheduled() const {
- return all_of(Bundle,
- [](const ScheduleData *SD) { return SD->isScheduled(); });
- }
-
/// Returns the number of unscheduled dependencies in the bundle.
int unscheduledDepsInBundle() const {
assert(*this && "bundle must not be empty");
@@ -4814,12 +4803,19 @@ class BoUpSLP {
ProcessBundleMember(SD, nullptr);
} else {
ScheduleBundle &Bundle = *cast<ScheduleBundle>(Data);
- for_each(Bundle.getBundle(), [](ScheduleData *SD) {
- SD->setScheduled(/*Scheduled=*/true);
- });
+ Bundle.setScheduled(/*Scheduled=*/true);
LLVM_DEBUG(dbgs() << "SLP: schedule " << Bundle << "\n");
- for (ScheduleData *SD : Bundle.getBundle())
- ProcessBundleMember(SD, &Bundle);
+ for (ScheduleData *SD : Bundle.getBundle()) {
+ if (ArrayRef<ScheduleBundle *> SDBundles =
+ getScheduleBundles(SD->getInst());
+ !SDBundles.empty() &&
+ all_of(SDBundles, [&](const ScheduleBundle *SDBundle) {
+ return SDBundle->isScheduled();
+ })) {
+ SD->setScheduled(/*Scheduled=*/true);
+ ProcessBundleMember(SD, &Bundle);
+ }
+ }
}
}
@@ -4851,7 +4847,8 @@ class BoUpSLP {
}
for (const ScheduleEntity *Bundle : ReadyInsts) {
- assert(Bundle->isReady() && "item in ready list not ready?");
+ assert((Bundle->isReady() || Bundle->isScheduled()) &&
+ "item in ready list not ready?");
(void)Bundle;
}
}
@@ -7553,7 +7550,7 @@ void BoUpSLP::buildExternalUses(
// Some in-tree scalars will remain as scalar in vectorized
// instructions. If that is the case, the one in FoundLane will
// be used.
- if (any_of(UseEntries, [&](TreeEntry *UseEntry) {
+ if (all_of(UseEntries, [&](TreeEntry *UseEntry) {
return UseEntry->State == TreeEntry::ScatterVectorize ||
!doesInTreeUserNeedToExtract(
Scalar, getRootEntryInstruction(*UseEntry), TLI,
@@ -9567,14 +9564,34 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
// We now know that this is a vector of instructions of the same type from
// the same block.
- // Check that none of the instructions in the bundle are already in the tree.
- for (Value *V : VL) {
- if ((!IsScatterVectorizeUserTE && !isa<Instruction>(V)) ||
- doesNotNeedToBeScheduled(V))
- continue;
- if (isVectorized(V)) {
- LLVM_DEBUG(dbgs() << "SLP: The instruction (" << *V
- << ") is already in tree.\n");
+ // Check that none of the instructions in the bundle are already in the tree
+ // and the node may be not profitable for the vectorization as the small
+ // alternate node.
+ if (S && S.isAltShuffle()) {
+ unsigned NumVectorized = 0;
+ unsigned NumExtracted = 0;
+ for (Value *V : VL) {
+ auto *I = dyn_cast<Instruction>(V);
+ if (!I || doesNotNeedToBeScheduled(V) ||
+ all_of(I->operands(), [&](const Use &U) {
+ return isa<ExtractElementInst>(U.get());
+ }))
+ continue;
+ if (isVectorized(V))
+ ++NumVectorized;
+ else if (!V->hasOneUser() && !areAllUsersVectorized(I, UserIgnoreList))
+ ++NumExtracted;
+ }
+ constexpr TTI::TargetCostKind Kind = TTI::TCK_RecipThroughput;
+ if (NumVectorized > 0 &&
+ (VL.size() == 2 ||
+ (getShuffleCost(*TTI, TTI::SK_PermuteTwoSrc,
+ getWidenedType(VL.front()->getType(), VL.size()), {},
+ Kind) +
+ NumExtracted >
+ VL.size() - NumVectorized))) {
+ LLVM_DEBUG(dbgs() << "SLP: The instructions are in tree and alternate "
+ "node is not profitable.\n");
if (TryToFindDuplicates(S)) {
auto Invalid = ScheduleBundle::invalid();
newTreeEntry(VL, Invalid /*not vectorized*/, S, UserTreeIdx,
@@ -9663,8 +9680,6 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
#endif
if (!BundlePtr || (*BundlePtr && !*BundlePtr.value())) {
LLVM_DEBUG(dbgs() << "SLP: We are not able to schedule this bundle!\n");
- assert((!BS.getScheduleData(VL0) || BS.getScheduleBundles(VL0).empty()) &&
- "tryScheduleBundle should not create bundle on failure");
// Last chance to try to vectorize alternate node.
if (S.isAltShuffle() && ReuseShuffleIndices.empty() &&
TrySplitNode(SmallNodeSize, S))
@@ -12443,7 +12458,7 @@ BoUpSLP::getEntryCost(const TreeEntry *E, ArrayRef<Value *> VectorizedVals,
SmallBitVector UsedScalars(Sz, false);
for (unsigned I = 0; I < Sz; ++I) {
if (isa<Instruction>(UniqueValues[I]) &&
- is_contained(getTreeEntries(UniqueValues[I]), E))
+ getTreeEntries(UniqueValues[I]).front() == E)
continue;
UsedScalars.set(I);
}
@@ -13971,6 +13986,7 @@ InstructionCost BoUpSLP::getTreeCost(ArrayRef<Value *> VectorizedVals,
for (ExternalUser &EU : ExternalUses) {
ScalarUserAndIdx.emplace_back(EU.Scalar, EU.User, EU.Lane);
}
+ SmallDenseSet<std::pair<Value *, Value *>, 8> CheckedScalarUser;
for (ExternalUser &EU : ExternalUses) {
// Uses by ephemeral values are free (because the ephemeral value will be
// removed prior to code generation, and so the extraction will be
@@ -13978,6 +13994,12 @@ InstructionCost BoUpSLP::getTreeCost(ArrayRef<Value *> VectorizedVals,
if (EphValues.count(EU.User))
continue;
+ // Check if the scalar for the given user or all users is accounted already.
+ if (!CheckedScalarUser.insert(std::make_pair(EU.Scalar, EU.User)).second ||
+ (EU.User &&
+ CheckedScalarUser.contains(std::make_pair(EU.Scalar, nullptr))))
+ continue;
+
// Used in unreachable blocks or in EH pads (rarely executed) or is
// terminated with unreachable instruction.
if (BasicBlock *UserParent =
@@ -14680,10 +14702,16 @@ BoUpSLP::isGatherShuffledSingleRegisterEntry(
PHINode *UserPHI = UseEI.UserTE->State != TreeEntry::SplitVectorize
? dyn_cast<PHINode>(UseEI.UserTE->getMainOp())
: nullptr;
- const Instruction *InsertPt =
+ Instruction *InsertPt =
UserPHI ? UserPHI->getIncomingBlock(UseEI.EdgeIdx)->getTerminator()
: &getLastInstructionInBundle(UseEI.UserTE);
if (TEInsertPt == InsertPt) {
+ // If the schedulable insertion point is used in multiple entries - just
+ // exit, no known ordering at this point, available only after real
+ // scheduling.
+ if (!doesNotNeedToBeScheduled(InsertPt) &&
+ (TEUseEI.UserTE != UseEI.UserTE || TEUseEI.EdgeIdx < UseEI.EdgeIdx))
+ continue;
// If the users are the PHI nodes with the same incoming blocks - skip.
if (TEUseEI.UserTE->State == TreeEntry::Vectorize &&
TEUseEI.UserTE->getOpcode() == Instruction::PHI &&
@@ -15395,19 +15423,29 @@ Instruction &BoUpSLP::getLastInstructionInBundle(const TreeEntry *E) {
// Set the insert point to the beginning of the basic block if the entry
// should not be scheduled.
- const auto *It = BlocksSchedules.find(BB);
- auto IsNotScheduledEntry = [&](const TreeEntry *E) {
+ auto FindScheduleBundle = [&](const TreeEntry *E) -> const ScheduleBundle * {
if (E->isGather())
- return false;
+ return nullptr;
// Found previously that the instruction do not need to be scheduled.
- return It == BlocksSchedules.end() || all_of(E->Scalars, [&](Value *V) {
- if (!isa<Instruction>(V))
- return true;
- return It->second->getScheduleBundles(V).empty();
- });
+ const auto *It = BlocksSchedules.find(BB);
+ if (It == BlocksSchedules.end())
+ return nullptr;
+ for (Value *V : E->Scalars) {
+ auto *I = dyn_cast<Instruction>(V);
+ if (!I || isa<PHINode>(I) || doesNotNeedToBeScheduled(I))
+ continue;
+ ArrayRef<ScheduleBundle *> Bundles = It->second->getScheduleBundles(I);
+ if (Bundles.empty())
+ continue;
+ const auto *It = find_if(
+ Bundles, [&](ScheduleBundle *B) { return B->getTreeEntry() == E; });
+ if (It != Bundles.end())
+ return *It;
+ }
+ return nullptr;
};
- if (IsNotScheduledEntry(E) ||
- (!E->isGather() && all_of(E->Scalars, isVectorLikeInstWithConstOps))) {
+ const ScheduleBundle *Bundle = FindScheduleBundle(E);
+ if (!E->isGather() && !Bundle) {
if ((E->getOpcode() == Instruction::GetElementPtr &&
any_of(E->Scalars,
[](Value *V) {
@@ -15433,19 +15471,10 @@ Instruction &BoUpSLP::getLastInstructionInBundle(const TreeEntry *E) {
// scheduled, and the last instruction is VL.back(). So we start with
// VL.back() and iterate over schedule data until we reach the end of the
// bundle. The end of the bundle is marked by null ScheduleData.
- if (It != BlocksSchedules.end() && !E->isGather()) {
- Value *V = E->isOneOf(E->Scalars.back());
- if (doesNotNeedToBeScheduled(V))
- V = *find_if_not(E->Scalars, doesNotNeedToBeScheduled);
- if (ArrayRef<ScheduleBundle *> Bundles = It->second->getScheduleBundles(V);
- !Bundles.empty()) {
- const auto *It = find_if(
- Bundles, [&](ScheduleBundle *B) { return B->getTreeEntry() == E; });
- assert(It != Bundles.end() && "Failed to find bundle");
- Res = (*It)->getBundle().back()->getInst();
- return *Res;
- }
- assert(E->getOpcode() == Instruction::PHI && "Expected PHI");
+ if (Bundle) {
+ assert(!E->isGather() && "Gathered instructions should not be scheduled");
+ Res = Bundle->getBundle().back()->getInst();
+ return *Res;
}
// LastInst can still be null at this point if there's either not an entry
@@ -17851,13 +17880,13 @@ Value *BoUpSLP::vectorizeTree(
const ExtraValueToDebugLocsMap &ExternallyUsedValues,
Instruction *ReductionRoot,
ArrayRef<std::tuple<Value *, unsigned, bool>> VectorValuesAndScales) {
+ // Clean Entry-to-LastInstruction table. It can be affected after scheduling,
+ // need to rebuild it.
+ EntryToLastInstruction.clear();
// All blocks must be scheduled before any instructions are inserted.
for (auto &BSIter : BlocksSchedules) {
scheduleBlock(BSIter.second.get());
}
- // Clean Entry-to-LastInstruction table. It can be affected after scheduling,
- // need to rebuild it.
- EntryToLastInstruction.clear();
if (ReductionRoot)
Builder.SetInsertPoint(ReductionRoot->getParent(),
@@ -18696,18 +18725,15 @@ BoUpSLP::BlockScheduling::tryScheduleBundle(ArrayRef<Value *> VL, BoUpSLP *SLP,
// dependencies. As soon as the bundle is "ready" it means that there are no
// cyclic dependencies and we can schedule it. Note that's important that we
// don't "schedule" the bundle yet.
- SmallPtrSet<const ScheduleBundle *, 16> Visited;
while (((!Bundle && ReSchedule) || (Bundle && !Bundle.isReady())) &&
!ReadyInsts.empty()) {
ScheduleEntity *Picked = ReadyInsts.pop_back_val();
- const auto *PickedBundle = dyn_cast<ScheduleBundle>(Picked);
- if (PickedBundle && !Visited.insert(PickedBundle).second) {
- assert(PickedBundle->isScheduled() && "bundle must be scheduled");
+ if (Picked->isScheduled()) {
+ if (Picked == &Bundle)
+ break;
continue;
}
- assert((PickedBundle ? PickedBundle->isReady()
- : cast<ScheduleData>(Picked)->isReady()) &&
- "must be ready to schedule");
+ assert(Picked->isReady() && "must be ready to schedule");
schedule(Picked, ReadyInsts);
if (Picked == &Bundle)
break;
@@ -18761,8 +18787,16 @@ BoUpSLP::BlockScheduling::tryScheduleBundle(ArrayRef<Value *> VL, BoUpSLP *SLP,
TryScheduleBundleImpl(ReSchedule, Bundle);
if (!Bundle.isReady()) {
for (ScheduleData *BD : Bundle.getBundle()) {
- if (BD->isReady())
- ReadyInsts.insert(BD);
+ if (BD->isReady()) {
+ ArrayRef<ScheduleBundle *> Bundles = getScheduleBundles(BD->getInst());
+ if (Bundles.empty()) {
+ ReadyInsts.insert(BD);
+ continue;
+ }
+ for (ScheduleBundle *B : Bundles)
+ if (B->isReady())
+ ReadyInsts.insert(B);
+ }
}
ScheduledBundlesList.pop_back();
for (Value *V : VL) {
@@ -19093,6 +19127,11 @@ void BoUpSLP::BlockScheduling::resetSchedule() {
SD->setScheduled(/*Scheduled=*/false);
SD->resetUnscheduledDeps();
}
+ for (ScheduleBundle *Bundle: getScheduleBundles(I)) {
+ assert(isInSchedulingRegion(*Bundle) &&
+ "ScheduleBundle not in scheduling region");
+ Bundle->setScheduled(/*Scheduled=*/false);
+ }
}
ReadyInsts.clear();
}
@@ -19151,6 +19190,7 @@ void BoUpSLP::scheduleBlock(BlockScheduling *BS) {
Instruction *LastScheduledInst = BS->ScheduleEnd;
// Do the "real" scheduling.
+ SmallPtrSet<Instruction *, 16> Scheduled;
while (!ReadyInsts.empty()) {
auto *Picked = *ReadyInsts.begin();
ReadyInsts.erase(ReadyInsts.begin());
@@ -19160,10 +19200,14 @@ void BoUpSLP::scheduleBlock(BlockScheduling *BS) {
if (auto *Bundle = dyn_cast<ScheduleBundle>(Picked)) {
for (const ScheduleData *BundleMember : Bundle->getBundle()) {
Instruction *PickedInst = BundleMember->getInst();
+ if (!Scheduled.insert(PickedInst).second)
+ continue;
if (PickedInst->getNextNonDebugInstruction() != LastScheduledInst)
PickedInst->moveAfter(LastScheduledInst->getPrevNode());
LastScheduledInst = PickedInst;
}
+ EntryToLastInstruction.try_emplace(Bundle->getTreeEntry(),
+ LastScheduledInst);
} else {
auto *SD = cast<ScheduleData>(Picked);
Instruction *PickedInst = SD->getInst();
diff --git a/llvm/test/Transforms/SLPVectorizer/AArch64/reused-scalar-repeated-in-node.ll b/llvm/test/Transforms/SLPVectorizer/AArch64/reused-scalar-repeated-in-node.ll
index 3cab4a4da3f8e..fcd3bfc3f323a 100644
--- a/llvm/test/Transforms/SLPVectorizer/AArch64/reused-scalar-repeated-in-node.ll
+++ b/llvm/test/Transforms/SLPVectorizer/AArch64/reused-scalar-repeated-in-node.ll
@@ -39,28 +39,26 @@ define void @test() {
; CHECK: [[BB77]]:
; CHECK-NEXT: [[TMP12:%.*]] = shufflevector <16 x float> [[TMP11]], <16 x float> poison, <8 x i32> <i32 poison, i32 poison, i32 poison, i32 poison, i32 14, i32 15, i32 poison, i32 poison>
; CHECK-NEXT: [[TMP17:%.*]] = insertelement <8 x float> [[TMP12]], float [[I70]], i32 0
-; CHECK-NEXT: [[TMP30:%.*]] = insertelement <2 x float> poison, float [[I68]], i32 0
-; CHECK-NEXT: [[TMP31:%.*]] = insertelement <2 x float> [[TMP30]], float [[I66]], i32 1
+; CHECK-NEXT: [[TMP14:%.*]] = insertelement <8 x float> poison, float [[I70]], i32 1
+; CHECK-NEXT: [[TMP19:%.*]] = insertelement <8 x float> [[TMP14]], float [[I68]], i32 2
+; CHECK-NEXT: [[TMP16:%.*]] = insertelement <8 x float> [[TMP19]], float [[I66]], i32 3
+; CHECK-NEXT: [[TMP20:%.*]] = insertelement <8 x float> [[TMP16]], float [[I67]], i32 6
+; CHECK-NEXT: [[TMP21:%.*]] = insertelement <8 x float> [[TMP20]], float [[I69]], i32 7
; CHECK-NEXT: [[TMP39:%.*]] = shufflevector <16 x float> [[TMP25]], <16 x float> poison, <16 x i32> <i32 poison, i32 poison, i32 3, i32 2, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
; CHECK-NEXT: [[TMP13:%.*]] = shufflevector <16 x float> [[TMP39]], <16 x float> [[TMP25]], <16 x i32> <i32 poison, i32 poison, i32 2, i32 3, i32 18, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 19, i32 poison, i32 poison>
; CHECK-NEXT: br label %[[BB78:.*]]
; CHECK: [[BB78]]:
; CHECK-NEXT: [[TMP15:%.*]] = phi <8 x float> [ [[TMP17]], %[[BB77]] ], [ [[TMP36:%.*]], %[[BB78]] ]
-; CHECK-NEXT: [[TMP16:%.*]] = phi <2 x float> [ [[TMP31]], %[[BB77]] ], [ [[TMP37:%.*]], %[[BB78]] ]
+; CHECK-NEXT: [[TMP22:%.*]] = phi <8 x float> [ [[TMP21]], %[[BB77]] ], [ [[TMP31:%.*]], %[[BB78]] ]
+; CHECK-NEXT: [[TMP24:%.*]] = shufflevector <8 x float> [[TMP22]], <8 x float> poison, <16 x i32> <i32 0, i32 3, i32 1, i32 2, i32 3, i32 0, i32 2, i32 3, i32 2, i32 6, i32 2, i32 3, i32 0, i32 7, i32 6, i32 6>
; CHECK-NEXT: [[TMP38:%.*]] = shufflevector <8 x float> [[TMP15]], <8 x float> poison, <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 1, i32 0, i32 3, i32 1, i32 3, i32 5, i32 3, i32 1, i32 0, i32 4, i32 5, i32 5>
-; CHECK-NEXT: [[TMP21:%.*]] = shufflevector <8 x float> [[TMP15]], <8 x float> poison, <16 x i32> <i32 2, i32 poison, i32 0, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
-; CHECK-NEXT: [[TMP20:%.*]] = shufflevector <2 x float> [[TMP16]], <2 x float> poison, <16 x i32> <i32 0, i32 1, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
-; CHECK-NEXT: [[TMP23:%.*]] = shufflevector <16 x float> [[TMP21]], <16 x float> [[TMP20]], <16 x i32> <i32 0, i32 17, i32 2, i32 16, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
-; CHECK-NEXT: [[TMP22:%.*]] = shufflevector <8 x float> [[TMP15]], <8 x float> poison, <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
-; CHECK-NEXT: [[TMP40:%.*]] = shufflevector <16 x float> [[TMP23]], <16 x float> [[TMP22]], <16 x i32> <i32 0, i32 1, i32...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Created using spr 1.3.5
RKSimon
reviewed
Mar 26, 2025
RKSimon
reviewed
Mar 26, 2025
gbossu
reviewed
Mar 27, 2025
| @@ -7553,7 +7550,7 @@ void BoUpSLP::buildExternalUses( | |||
| // Some in-tree scalars will remain as scalar in vectorized | |||
| // instructions. If that is the case, the one in FoundLane will | |||
| // be used. | |||
| if (any_of(UseEntries, [&](TreeEntry *UseEntry) { | |||
| if (all_of(UseEntries, [&](TreeEntry *UseEntry) { | |||
There was a problem hiding this comment.
Checking: After changing any_of to all_of, is the comment higher up still up to date?
There was a problem hiding this comment.
Yes, still correct. It is not related to the check itself, just describes common logic, which is not changed. Just before UseEntries could be only one, now there might be multiple UseEntries
Created using spr 1.3.5
hiraditya
reviewed
Mar 27, 2025
|
Ping! |
|
LGTM, unless @RKSimon has any more feedback. |
hiraditya
approved these changes
Mar 31, 2025
RKSimon
reviewed
Mar 31, 2025
Created using spr 1.3.5
Created using spr 1.3.5
gbossu
reviewed
Apr 1, 2025
| continue; | ||
| if (isVectorized(V)) | ||
| Vectorized.clearBit(Idx); | ||
| else if (!V->hasOneUser() && !areAllUsersVectorized(I, UserIgnoreList)) |
There was a problem hiding this comment.
Super-Nit: Given that you have auto *I = dyn_cast<Instruction>(V);, maybe use I everywhere, instead of sometimes V and sometimes I?
Created using spr 1.3.5
gbossu
approved these changes
Apr 1, 2025
Created using spr 1.3.5
alexey-bataev
deleted the
users/alexey-bataev/spr/slpsupport-revectorization-of-the-previously-vectorized-scalars
branch
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Apr 1, 2025
…scalars If the scalar instructions is marked for the vectorization in the tree, it cannot be vectorized as part of the another node in the same tree, in general. It may prevent some potentially profitable vectorization opportunities, since some nodes end up being buildvector/gather nodes, which add to the total cost. Patch allows revectorization of the previously vectorized scalars. Reviewers: hiraditya, RKSimon Reviewed By: RKSimon, hiraditya Pull Request: llvm/llvm-project#133091
Ankur-0429
pushed a commit
to Ankur-0429/llvm-project
that referenced
this pull request
Apr 2, 2025
If the scalar instructions is marked for the vectorization in the tree, it cannot be vectorized as part of the another node in the same tree, in general. It may prevent some potentially profitable vectorization opportunities, since some nodes end up being buildvector/gather nodes, which add to the total cost. Patch allows revectorization of the previously vectorized scalars. Reviewers: hiraditya, RKSimon Reviewed By: RKSimon, hiraditya Pull Request: llvm#133091
|
Hi @alexey-bataev, we see Clang crashes after 0e3049c. I'm working on a reduce test case, but please revert in the meantime. |
|
Test case (also in https://gcc.godbolt.org/z/nsjaqfoT8): |
|
Sent #134604. I'll land it as soon as the premerge checks finish. |
Thanks for the reproducer! |
alexey-bataev
added a commit
that referenced
this pull request
Apr 7, 2025
…n SplitVectorize nodes If the last instruction in the SplitVectorize node is vectorized and scheduled as part of some bundles, the SplitVectorize node might be placed in the wrong order, leading to a compiler crash. Need to check if the vectorized node has vector value and place the SplitVectorize node after the vector instruction to prevent a compile crash. Fixes issue reported in #133091 (comment)
Fixed in f413772 |
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Apr 7, 2025
…struction in SplitVectorize nodes If the last instruction in the SplitVectorize node is vectorized and scheduled as part of some bundles, the SplitVectorize node might be placed in the wrong order, leading to a compiler crash. Need to check if the vectorized node has vector value and place the SplitVectorize node after the vector instruction to prevent a compile crash. Fixes issue reported in llvm/llvm-project#133091 (comment)
Thanks! Will try with the original failures. |
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.
If the scalar instructions is marked for the vectorization in the tree,
it cannot be vectorized as part of the another node in the same tree, in
general. It may prevent some potentially profitable vectorization
opportunities, since some nodes end up being buildvector/gather nodes,
which add to the total cost.
Patch allows revectorization of the previously vectorized scalars.