[LV] Split checking if tail-folding is possible, collecting masked ops. #77612
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
|
@llvm/pr-subscribers-llvm-transforms Author: Florian Hahn (fhahn) ChangesIntroduce new canFoldTail helper which only checks if tail-folding is possible, but without modifying MaskedOps. Just because tail-folding is possible doesn't mean the tail will be folded; that's up to the cost-model to decide. Separating the check if tail-folding is possible and preparing for tail-folding makes sure that MaskedOps is only populated when tail-folding is actually selected. This allows only creating the header mask if needed after #76635. Full diff: https://github.com/llvm/llvm-project/pull/77612.diff 3 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h b/llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h
index a509ebf6a7e1b3..af23813da569b5 100644
--- a/llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h
+++ b/llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h
@@ -276,9 +276,12 @@ class LoopVectorizationLegality {
bool canVectorizeFPMath(bool EnableStrictReductions);
/// Return true if we can vectorize this loop while folding its tail by
- /// masking, and mark all respective loads/stores for masking.
- /// This object's state is only modified iff this function returns true.
- bool prepareToFoldTailByMasking();
+ /// masking.
+ bool canFoldTailByMasking() const;
+
+ /// Mark all respective loads/stores for masking. Must only be called when
+ /// ail-folding is possible.
+ void prepareToFoldTailByMasking();
/// Returns the primary induction variable.
PHINode *getPrimaryInduction() { return PrimaryInduction; }
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
index 37a356c43e29a4..88fabf620a83ab 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
@@ -1525,7 +1525,7 @@ bool LoopVectorizationLegality::canVectorize(bool UseVPlanNativePath) {
return Result;
}
-bool LoopVectorizationLegality::prepareToFoldTailByMasking() {
+bool LoopVectorizationLegality::canFoldTailByMasking() const {
LLVM_DEBUG(dbgs() << "LV: checking if tail can be folded by masking.\n");
@@ -1570,8 +1570,24 @@ bool LoopVectorizationLegality::prepareToFoldTailByMasking() {
LLVM_DEBUG(dbgs() << "LV: can fold tail by masking.\n");
- MaskedOp.insert(TmpMaskedOp.begin(), TmpMaskedOp.end());
return true;
}
+void LoopVectorizationLegality::prepareToFoldTailByMasking() {
+ // The list of pointers that we can safely read and write to remains empty.
+ SmallPtrSet<Value *, 8> SafePointers;
+
+ // Collect masked ops in temporary set first to avoid partially populating
+ // MaskedOp if a block cannot be predicated.
+ SmallPtrSet<const Instruction *, 8> TmpMaskedOp;
+
+ // Check and mark all blocks for predication, including those that ordinarily
+ // do not need predication such as the header block.
+ for (BasicBlock *BB : TheLoop->blocks()) {
+ bool R = blockCanBePredicated(BB, SafePointers, MaskedOp);
+ (void)R;
+ assert(R && "Must be able to predicate block when tail-folding.");
+ }
+}
+
} // namespace llvm
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index 51ce88480c0880..d2506bd2aee6ae 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -4689,7 +4689,7 @@ LoopVectorizationCostModel::computeMaxVF(ElementCount UserVF, unsigned UserIC) {
// found modulo the vectorization factor is not zero, try to fold the tail
// by masking.
// FIXME: look for a smaller MaxVF that does divide TC rather than masking.
- if (Legal->prepareToFoldTailByMasking()) {
+ if (Legal->canFoldTailByMasking()) {
CanFoldTailByMasking = true;
return MaxFactors;
}
@@ -7307,6 +7307,9 @@ LoopVectorizationPlanner::plan(ElementCount UserVF, unsigned UserIC) {
CM.invalidateCostModelingDecisions();
}
+ if (CM.foldTailByMasking())
+ Legal->prepareToFoldTailByMasking();
+
ElementCount MaxUserVF =
UserVF.isScalable() ? MaxFactors.ScalableVF : MaxFactors.FixedVF;
bool UserVFIsLegal = ElementCount::isKnownLE(UserVF, MaxUserVF);
@@ -8680,10 +8683,12 @@ LoopVectorizationPlanner::tryToBuildVPlanWithVPRecipes(VFRange &Range) {
VPBB->setName(BB->getName());
Builder.setInsertPoint(VPBB);
- if (VPBB == HeaderVPBB)
- RecipeBuilder.createHeaderMask(*Plan);
- else if (NeedsMasks)
- RecipeBuilder.createBlockInMask(BB, *Plan);
+ if (NeedsMasks) {
+ if (VPBB == HeaderVPBB)
+ RecipeBuilder.createHeaderMask(*Plan);
+ else
+ RecipeBuilder.createBlockInMask(BB, *Plan);
+ }
// Introduce each ingredient into VPlan.
// TODO: Model and preserve debug intrinsics in VPlan.
|
fhahn
force-pushed
the
split-can-fold-tail-prepare-for-tailfolding
branch
from
507e366 to
35741d9
Compare
fhahn
force-pushed
the
split-can-fold-tail-prepare-for-tailfolding
branch
from
35741d9 to
2aaf792
Compare
Introduce new canFoldTail helper which only checks if tail-folding is possible, but without modifying MaskedOps. Just because tail-folding is possible doesn't mean the tail will be folded; that's up to the cost-model to decide. Separating the check if tail-folding is possible and preparing for tail-folding makes sure that MaskedOps is only populated when tail-folding is actually selected. This allows only creating the header mask if needed after llvm#76635.
fhahn
force-pushed
the
split-can-fold-tail-prepare-for-tailfolding
branch
from
2aaf792 to
9fa9457
Compare
|
Updated |
alexey-bataev
approved these changes
Jul 8, 2024
Comment on lines
1618
to
1619
| bool R = blockCanBePredicated(BB, SafePointers, MaskedOp); | ||
| (void)R; |
There was a problem hiding this comment.
Suggested change
| bool R = blockCanBePredicated(BB, SafePointers, MaskedOp); | |
| (void)R; | |
| [[maybe_unused]] bool R = blockCanBePredicated(BB, SafePointers, MaskedOp); |
ayalz
reviewed
Jul 8, 2024
| bool canFoldTailByMasking() const; | ||
|
|
||
| /// Mark all respective loads/stores for masking. Must only be called when | ||
| /// ail-folding is possible. |
There was a problem hiding this comment.
Suggested change
| /// ail-folding is possible. | |
| /// tail-folding is possible. |
| return true; | ||
| } | ||
|
|
||
| void LoopVectorizationLegality::prepareToFoldTailByMasking() { |
There was a problem hiding this comment.
Above comments worth updating:
// Collect masked ops in temporary set first to avoid partially populating
// MaskedOp if a block cannot be predicated.
comment needs rephrasing.
// Check and mark all blocks for predication, including those that ordinarily
// do not need predication such as the header block.
only Check, w/o mark
LLVM_DEBUG(dbgs() << "LV: Cannot fold tail by masking as requested.\n");
drop "as requested"?
Comment on lines
1611
to
1614
| // Collect masked ops in temporary set first to avoid partially populating | ||
| // MaskedOp if a block cannot be predicated. | ||
| SmallPtrSet<const Instruction *, 8> TmpMaskedOp; | ||
|
|
There was a problem hiding this comment.
Suggested change
| // Collect masked ops in temporary set first to avoid partially populating | |
| // MaskedOp if a block cannot be predicated. | |
| SmallPtrSet<const Instruction *, 8> TmpMaskedOp; |
| // MaskedOp if a block cannot be predicated. | ||
| SmallPtrSet<const Instruction *, 8> TmpMaskedOp; | ||
|
|
||
| // Check and mark all blocks for predication, including those that ordinarily |
There was a problem hiding this comment.
Suggested change
| // Check and mark all blocks for predication, including those that ordinarily | |
| // Mark all blocks for predication, including those that ordinarily |
| // Check and mark all blocks for predication, including those that ordinarily | ||
| // do not need predication such as the header block. | ||
| for (BasicBlock *BB : TheLoop->blocks()) { | ||
| bool R = blockCanBePredicated(BB, SafePointers, MaskedOp); |
There was a problem hiding this comment.
Worth considering splitting this into two, as checking if block can be predicated and collecting its masked ops (if it can) seem two distinct operations. Both avoid considering SafePointers.
There was a problem hiding this comment.
Left as is for now for this PR to keep the number of changes small, but will check as follow-up. More generally, we should probably collect and consider safe pointers with tail folding as well
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/41/builds/456 Here is the relevant piece of the build log for the reference: |
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.
Introduce new canFoldTail helper which only checks if tail-folding is possible, but without modifying MaskedOps.
Just because tail-folding is possible doesn't mean the tail will be folded; that's up to the cost-model to decide. Separating the check if tail-folding is possible and preparing for tail-folding makes sure that MaskedOps is only populated when tail-folding is actually selected.
This allows only creating the header mask if needed after #76635.