Skip to content

[LV] Use llvm::all_of in LoopVectorizationCostModel::getMaximizedVFForTarget. NFC #99585

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
Jul 20, 2024

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Jul 18, 2024

No description provided.

@topperc topperc requested review from fhahn and npanchen July 18, 2024 23:47
@npanchen
Copy link
Contributor

add NFC to title?

@llvmbot
Copy link
Member

llvmbot commented Jul 19, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Craig Topper (topperc)

Changes

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

1 Files Affected:

  • (modified) llvm/lib/Transforms/Vectorize/LoopVectorize.cpp (+6-9)
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index fbca4cdcbcfcd..cf5b55191527b 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -4589,15 +4589,12 @@ ElementCount LoopVectorizationCostModel::getMaximizedVFForTarget(
 
     // Select the largest VF which doesn't require more registers than existing
     // ones.
-    for (int i = RUs.size() - 1; i >= 0; --i) {
-      bool Selected = true;
-      for (auto &pair : RUs[i].MaxLocalUsers) {
-        unsigned TargetNumRegisters = TTI.getNumberOfRegisters(pair.first);
-        if (pair.second > TargetNumRegisters)
-          Selected = false;
-      }
-      if (Selected) {
-        MaxVF = VFs[i];
+    for (int I = RUs.size() - 1; I >= 0; --I) {
+      const auto &MLU = RUs[I].MaxLocalUsers;
+      if (llvm::all_of(MLU, [&](decltype(MLU.front()) &LU) {
+            return LU.second <= TTI.getNumberOfRegisters(LU.first);
+          })) {
+        MaxVF = VFs[I];
         break;
       }
     }

@topperc topperc changed the title [LV] Use llvm::all_off in LoopVectorizationCostModel::getMaximizedVFForTarget. NFC [LV] Use llvm::all_of in LoopVectorizationCostModel::getMaximizedVFForTarget. NFC Jul 19, 2024
@topperc
Copy link
Collaborator Author

topperc commented Jul 19, 2024

add NFC to title?

Isn't it already at the end of the title?

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!

MaxVF = VFs[i];
for (int I = RUs.size() - 1; I >= 0; --I) {
const auto &MLU = RUs[I].MaxLocalUsers;
if (llvm::all_of(MLU, [&](decltype(MLU.front()) &LU) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: llvm:: should not be needed

@npanchen
Copy link
Contributor

add NFC to title?

Isn't it already at the end of the title?

my bad. Was looking at the beginning of the title

@topperc topperc merged commit be7f182 into llvm:main Jul 20, 2024
7 checks passed
@topperc topperc deleted the pr/all-of branch July 20, 2024 00:13
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
…rTarget. NFC (#99585)

Summary: 

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251578
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