Skip to content

Conversation

@Mel-Chen
Copy link
Contributor

@Mel-Chen Mel-Chen commented Oct 23, 2024

Iteration runtime check confirms whether the trip count is greater than VFxUF at least. Therefore, there is no need to adjust the MinProfitableTripCount to VF if it is zero.
Retaining the original MinProfitableTripCount information is also beneficial for supporting more profitable runtime checks in the future.

Iteration runtime check confirms whether the trip count is greater than
VFxUF at least. Therefore, there is no need to adjust the
MinProfitableTripCount to VFxUF if it is zero.

Retaining the original MinProfitableTripCount information is also
beneficial for supporting more profitable runtime checks in the future.
@llvmbot
Copy link
Member

llvmbot commented Oct 23, 2024

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-vectorizers

Author: Mel Chen (Mel-Chen)

Changes

Iteration runtime check confirms whether the trip count is greater than VFxUF at least. Therefore, there is no need to adjust the MinProfitableTripCount to VFxUF if it is zero.
Retaining the original MinProfitableTripCount information is also beneficial for supporting more profitable runtime checks in the future.


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

1 Files Affected:

  • (modified) llvm/lib/Transforms/Vectorize/LoopVectorize.cpp (+2-6)
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index 60a94ca1f86e42..804b541e8ffdec 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -469,18 +469,14 @@ class InnerLoopVectorizer {
                       LoopVectorizationCostModel *CM, BlockFrequencyInfo *BFI,
                       ProfileSummaryInfo *PSI, GeneratedRTChecks &RTChecks)
       : OrigLoop(OrigLoop), PSE(PSE), LI(LI), DT(DT), TLI(TLI), TTI(TTI),
-        AC(AC), ORE(ORE), VF(VecWidth), UF(UnrollFactor),
+        AC(AC), ORE(ORE), VF(VecWidth),
+        MinProfitableTripCount(MinProfitableTripCount), UF(UnrollFactor),
         Builder(PSE.getSE()->getContext()), Legal(LVL), Cost(CM), BFI(BFI),
         PSI(PSI), RTChecks(RTChecks) {
     // Query this against the original loop and save it here because the profile
     // of the original loop header may change as the transformation happens.
     OptForSizeBasedOnProfile = llvm::shouldOptimizeForSize(
         OrigLoop->getHeader(), PSI, BFI, PGSOQueryType::IRPass);
-
-    if (MinProfitableTripCount.isZero())
-      this->MinProfitableTripCount = VecWidth;
-    else
-      this->MinProfitableTripCount = MinProfitableTripCount;
   }
 
   virtual ~InnerLoopVectorizer() = default;

@Mel-Chen
Copy link
Contributor Author

Ping

@david-arm
Copy link
Contributor

On the surface the change seems fine. Do you have any examples of where this would be useful?

Copy link
Contributor

@fhahn fhahn left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! There should be no need to handle the case where it is zero separately.

@Mel-Chen
Copy link
Contributor Author

Mel-Chen commented Nov 5, 2024

On the surface the change seems fine. Do you have any examples of where this would be useful?

We encountered a case where vectorization requires a lot of overflow checks, resulting in the need for a higher trip count for vectorization to be profitable. We hope to emit a profitable check for such cases when MinProfitableTripCount is not zero, in order to reduce unprofitable vectorized code be executed.

@Mel-Chen Mel-Chen merged commit 70de0b8 into llvm:main Nov 5, 2024
7 checks passed
PhilippRados pushed a commit to PhilippRados/llvm-project that referenced this pull request Nov 6, 2024
)

Iteration runtime check confirms whether the trip count is greater than
VFxUF at least. Therefore, there is no need to adjust the
MinProfitableTripCount to VF if it is zero.
Retaining the original MinProfitableTripCount information is also
beneficial for supporting more profitable runtime checks in the future.
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.

4 participants