-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[LV][NFC]Introduce isScalableVectorizationAllowed() to refactor getMaxLegalScalableVF(). #98916
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Created using spr 1.3.5
@llvm/pr-subscribers-llvm-transforms Author: Alexey Bataev (alexey-bataev) ChangesAdds isScalableVectorizationAllowed() and the corresponding data member Part of #91403 Full diff: https://github.com/llvm/llvm-project/pull/98916.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index 5520baef7152d..86957af9ce6c7 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -1627,6 +1627,10 @@ class LoopVectorizationCostModel {
ElementCount MaxSafeVF,
bool FoldTailByMasking);
+ /// Checks if scalable vectorization is supported and enabled. Caches the
+ /// result to avoid repeated debug dumps for repeated queries.
+ bool isScalableVectorizationAllowed();
+
/// \return the maximum legal scalable VF, based on the safe max number
/// of elements.
ElementCount getMaxLegalScalableVF(unsigned MaxSafeElements);
@@ -1691,6 +1695,9 @@ class LoopVectorizationCostModel {
std::optional<std::pair<TailFoldingStyle, TailFoldingStyle>>
ChosenTailFoldingStyle;
+ /// true if scalable vectorization is supported and enabled.
+ std::optional<bool> IsScalableVectorizationAllowed;
+
/// A map holding scalar costs for different vectorization factors. The
/// presence of a cost for an instruction in the mapping indicates that the
/// instruction will be scalarized when vectorizing with the associated
@@ -4143,15 +4150,18 @@ bool LoopVectorizationCostModel::runtimeChecksRequired() {
return false;
}
-ElementCount
-LoopVectorizationCostModel::getMaxLegalScalableVF(unsigned MaxSafeElements) {
+bool LoopVectorizationCostModel::isScalableVectorizationAllowed() {
+ if (IsScalableVectorizationAllowed)
+ return *IsScalableVectorizationAllowed;
+
+ IsScalableVectorizationAllowed = false;
if (!TTI.supportsScalableVectors() && !ForceTargetSupportsScalableVectors)
- return ElementCount::getScalable(0);
+ return false;
if (Hints->isScalableVectorizationDisabled()) {
reportVectorizationInfo("Scalable vectorization is explicitly disabled",
"ScalableVectorizationDisabled", ORE, TheLoop);
- return ElementCount::getScalable(0);
+ return false;
}
LLVM_DEBUG(dbgs() << "LV: Scalable vectorization is available\n");
@@ -4171,7 +4181,7 @@ LoopVectorizationCostModel::getMaxLegalScalableVF(unsigned MaxSafeElements) {
"Scalable vectorization not supported for the reduction "
"operations found in this loop.",
"ScalableVFUnfeasible", ORE, TheLoop);
- return ElementCount::getScalable(0);
+ return false;
}
// Disable scalable vectorization if the loop contains any instructions
@@ -4183,17 +4193,32 @@ LoopVectorizationCostModel::getMaxLegalScalableVF(unsigned MaxSafeElements) {
reportVectorizationInfo("Scalable vectorization is not supported "
"for all element types found in this loop.",
"ScalableVFUnfeasible", ORE, TheLoop);
- return ElementCount::getScalable(0);
+ return false;
}
+ IsScalableVectorizationAllowed = true;
+ return true;
+}
+
+ElementCount
+LoopVectorizationCostModel::getMaxLegalScalableVF(unsigned MaxSafeElements) {
+ if (!isScalableVectorizationAllowed())
+ return ElementCount::getScalable(0);
+
+ auto MaxScalableVF = ElementCount::getScalable(
+ std::numeric_limits<ElementCount::ScalarTy>::max());
if (Legal->isSafeForAnyVectorWidth())
return MaxScalableVF;
+ std::optional<unsigned> MaxVScale = getMaxVScale(*TheFunction, TTI);
+ if (!MaxVScale) {
+ reportVectorizationInfo("The target does not provide maximum vscale value.",
+ "ScalableVFUnfeasible", ORE, TheLoop);
+ return ElementCount::getScalable(0);
+ }
+
// Limit MaxScalableVF by the maximum safe dependence distance.
- if (std::optional<unsigned> MaxVScale = getMaxVScale(*TheFunction, TTI))
- MaxScalableVF = ElementCount::getScalable(MaxSafeElements / *MaxVScale);
- else
- MaxScalableVF = ElementCount::getScalable(0);
+ MaxScalableVF = ElementCount::getScalable(MaxSafeElements / *MaxVScale);
if (!MaxScalableVF)
reportVectorizationInfo(
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM, thanks for splitting! Adding minor comments.
std::optional<unsigned> MaxVScale = getMaxVScale(*TheFunction, TTI); | ||
if (!MaxVScale) { | ||
reportVectorizationInfo("The target does not provide maximum vscale value.", | ||
"ScalableVFUnfeasible", ORE, TheLoop); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for separating this message from the one below!
Does this !MaxVScale case better belong earlier, in isScalableVectorizationAllowed()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, better to put the check there.
return ElementCount::getScalable(0); | ||
|
||
auto MaxScalableVF = ElementCount::getScalable( | ||
std::numeric_limits<ElementCount::ScalarTy>::max()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine, preserving current behavior. Would be good if some assert could be added, independently, to make sure that MaxSafeElements
corresponds to MaxScalableVF
if isSafeForAnyVectorWidth(), as the former is ignored in this case, which returns the latter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I said before, this requires looking into the internals of LAA. No idea how safely do it here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, probably we can just use auto MaxScalableVF = ElementCount::getScalable( MaxSafeElements);
here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanjs
Created using spr 1.3.5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Post commit nit.
return false; | ||
} | ||
|
||
if (!Legal->isSafeForAnyVectorWidth()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Post-commit: this stems from the original order of checks, but could the check if isSafeForAnyVectorWidth be dropped? Sounds redundant, or good to indicate why otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It affects the vectorizations. Need to check the scale limit only if !Legal->isSafeForAnyVectorWidth()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, so be it. The error message could perhaps be more accurate then, as lack of maximum vscale alone may not imply that scalable VF is unfeasible. And perhaps suffice to check if (!Legal->isSafeForAnyVectorWidth() && !getMaxVScale()).
Adds isScalableVectorizationAllowed() and the corresponding data member
to query if the scalable vectorization is supported rather than
performing the analysis each time the scalable vector factor is
requested.
Part of #91403