Skip to content

[LV] Consider EVL legality for TTI tail folding preference #144790

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

Merged
merged 2 commits into from
Jun 19, 2025

Conversation

preames
Copy link
Collaborator

@preames preames commented Jun 18, 2025

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Jun 18, 2025

@llvm/pr-subscribers-vectorizers

@llvm/pr-subscribers-llvm-transforms

Author: Philip Reames (preames)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/144790.diff

1 Files Affected:

  • (modified) llvm/lib/Transforms/Vectorize/LoopVectorize.cpp (+20-23)
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index 2f4416d2782e8..fdcc61a5205ce 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -1364,16 +1364,14 @@ class LoopVectorizationCostModel {
       return;
     }
 
-    if (!ForceTailFoldingStyle.getNumOccurrences()) {
-      ChosenTailFoldingStyle = {
-          TTI.getPreferredTailFoldingStyle(/*IVUpdateMayOverflow=*/true),
-          TTI.getPreferredTailFoldingStyle(/*IVUpdateMayOverflow=*/false)};
-      return;
-    }
+    // Default to TTI preference, but allow command line override.
+    ChosenTailFoldingStyle = {
+        TTI.getPreferredTailFoldingStyle(/*IVUpdateMayOverflow=*/true),
+        TTI.getPreferredTailFoldingStyle(/*IVUpdateMayOverflow=*/false)};
+    if (ForceTailFoldingStyle.getNumOccurrences())
+      ChosenTailFoldingStyle = {ForceTailFoldingStyle.getValue(),
+                                ForceTailFoldingStyle.getValue()};
 
-    // Set styles when forced.
-    ChosenTailFoldingStyle = {ForceTailFoldingStyle.getValue(),
-                              ForceTailFoldingStyle.getValue()};
     if (ForceTailFoldingStyle != TailFoldingStyle::DataWithEVL)
       return;
     // Override forced styles if needed.
@@ -1382,20 +1380,19 @@ class LoopVectorizationCostModel {
     bool EVLIsLegal = UserIC <= 1 && IsScalableVF &&
                       TTI.hasActiveVectorLength(0, nullptr, Align()) &&
                       !EnableVPlanNativePath;
-    if (!EVLIsLegal) {
-      // If for some reason EVL mode is unsupported, fallback to
-      // DataWithoutLaneMask to try to vectorize the loop with folded tail
-      // in a generic way.
-      ChosenTailFoldingStyle = {TailFoldingStyle::DataWithoutLaneMask,
-                                TailFoldingStyle::DataWithoutLaneMask};
-      LLVM_DEBUG(
-          dbgs()
-          << "LV: Preference for VP intrinsics indicated. Will "
-             "not try to generate VP Intrinsics "
-          << (UserIC > 1
-                  ? "since interleave count specified is greater than 1.\n"
-                  : "due to non-interleaving reasons.\n"));
-    }
+    if (EVLIsLegal)
+      return;
+    // If for some reason EVL mode is unsupported, fallback to
+    // DataWithoutLaneMask to try to vectorize the loop with folded tail
+    // in a generic way.
+    ChosenTailFoldingStyle = {TailFoldingStyle::DataWithoutLaneMask,
+                              TailFoldingStyle::DataWithoutLaneMask};
+    LLVM_DEBUG(
+        dbgs() << "LV: Preference for VP intrinsics indicated. Will "
+                  "not try to generate VP Intrinsics "
+               << (UserIC > 1
+                       ? "since interleave count specified is greater than 1.\n"
+                       : "due to non-interleaving reasons.\n"));
   }
 
   /// Returns true if all loop blocks should be masked to fold tail loop.

Comment on lines +1368 to +1370
ChosenTailFoldingStyle = {
TTI.getPreferredTailFoldingStyle(/*IVUpdateMayOverflow=*/true),
TTI.getPreferredTailFoldingStyle(/*IVUpdateMayOverflow=*/false)};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a test for this change, I assume

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no way to test this with in tree backends.

Copy link
Contributor

@lukel97 lukel97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@preames preames merged commit d8e6d74 into llvm:main Jun 19, 2025
5 of 7 checks passed
@preames preames deleted the pr-lv-evl-check-after-tti branch June 19, 2025 23:15
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jun 20, 2025

LLVM Buildbot has detected a new failure on builder lldb-aarch64-windows running on linaro-armv8-windows-msvc-05 while building llvm at step 6 "test".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/141/builds/9572

Here is the relevant piece of the build log for the reference
Step 6 (test) failure: build (failure)
...
PASS: lldb-unit :: ValueObject/./LLDBValueObjectTests.exe/11/12 (2237 of 2247)
PASS: lldb-unit :: ValueObject/./LLDBValueObjectTests.exe/2/12 (2238 of 2247)
PASS: lldb-unit :: ValueObject/./LLDBValueObjectTests.exe/3/12 (2239 of 2247)
PASS: lldb-unit :: ValueObject/./LLDBValueObjectTests.exe/4/12 (2240 of 2247)
PASS: lldb-unit :: ValueObject/./LLDBValueObjectTests.exe/5/12 (2241 of 2247)
PASS: lldb-unit :: ValueObject/./LLDBValueObjectTests.exe/6/12 (2242 of 2247)
PASS: lldb-unit :: ValueObject/./LLDBValueObjectTests.exe/7/12 (2243 of 2247)
PASS: lldb-unit :: ValueObject/./LLDBValueObjectTests.exe/8/12 (2244 of 2247)
PASS: lldb-unit :: ValueObject/./LLDBValueObjectTests.exe/9/12 (2245 of 2247)
PASS: lldb-unit :: tools/lldb-server/tests/./LLDBServerTests.exe/0/1 (2246 of 2247)
command timed out: 2400 seconds without output running [b'ninja', b'check-lldb'], attempting to kill
program finished with exit code 1
elapsedTime=5129.039465

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants