Skip to content
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

[SLP] Use ScopeExit to update Operands/PrevDist on all paths. (NFC) #83490

Merged
merged 3 commits into from
Mar 1, 2024

Conversation

fhahn
Copy link
Contributor

@fhahn fhahn commented Feb 29, 2024

Use ScopeExit to make sure Operands/PrevDist are updated on all paths in the loop. This makes it easier to ensure they are updated correctly if new early continues are added.

Split off from #83283

Use ScopeExit to make sure Operands/PrevDist are updated on all paths in
the loop. This makes it easier to ensure they are updated correctly if
new early continues are added.

Split off from llvm#83283
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 29, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Florian Hahn (fhahn)

Changes

Use ScopeExit to make sure Operands/PrevDist are updated on all paths in the loop. This makes it easier to ensure they are updated correctly if new early continues are added.

Split off from #83283


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

1 Files Affected:

  • (modified) llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp (+10-7)
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 94b7c4952f055e..1cdd466e88d420 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -21,6 +21,7 @@
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/PriorityQueue.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SetOperations.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallBitVector.h"
@@ -13891,12 +13892,17 @@ bool SLPVectorizerPass::vectorizeStores(ArrayRef<StoreInst *> Stores,
         if (Idx != Set.size() - 1)
           continue;
       }
-      if (Operands.size() <= 1) {
+      // Capturing structured bindings are a C++ 20 extension, so use a separate
+      // variable for now.
+      auto &DataVar = Data;
+      auto E = make_scope_exit([&]() {
         Operands.clear();
-        Operands.push_back(Stores[Data.first]);
-        PrevDist = Data.second;
+        Operands.push_back(Stores[DataVar.first]);
+        PrevDist = DataVar.second;
+      });
+
+      if (Operands.size() <= 1)
         continue;
-      }
 
       unsigned MaxVecRegSize = R.getMaxVecRegSize();
       unsigned EltSize = R.getVectorElementSize(Operands[0]);
@@ -13955,9 +13961,6 @@ bool SLPVectorizerPass::vectorizeStores(ArrayRef<StoreInst *> Stores,
         if (StartIdx >= Operands.size())
           break;
       }
-      Operands.clear();
-      Operands.push_back(Stores[Data.first]);
-      PrevDist = Data.second;
     }
   };
 

Comment on lines 13897 to 13898
auto &DataVar = Data;
auto E = make_scope_exit([&]() {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
auto &DataVar = Data;
auto E = make_scope_exit([&]() {
auto E = make_scope_exit([&, &DataVar = Data]() {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Much better, updated, thanks!

Copy link
Member

@alexey-bataev alexey-bataev left a comment

Choose a reason for hiding this comment

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

LG

@fhahn fhahn merged commit 6ecd261 into llvm:main Mar 1, 2024
3 of 4 checks passed
@fhahn fhahn deleted the slp-scope-exit branch March 1, 2024 14:30
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.

None yet

3 participants