[llvm] [SLP] More OOP to simplify vectorizeStores() (NFC) (PR #134605)

Gaƫtan Bossu via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 11 09:07:51 PDT 2025


================
@@ -20358,12 +20364,54 @@ struct RelatedStoreInsts {
     return Inserted ? std::nullopt : std::optional<unsigned>(It->second);
   }
 
+  StoreInst &getBaseStore() const { return *AllStores[BaseInstrIdx]; }
+  using DistToInstMap = std::map<int, unsigned>;
+  const DistToInstMap &getStores() const { return Instrs; }
+
+  /// Recompute the pointer distances to be based on \p NewBaseInstIdx.
+  /// Stores whose index is less than \p MinSafeIdx will be dropped.
+  void rebase(unsigned MinSafeIdx, unsigned NewBaseInstIdx,
+              int DistFromCurBase) {
+    DistToInstMap PrevSet = std::move(Instrs);
+    reset(NewBaseInstIdx);
+
+    // Re-insert stores that come after MinSafeIdx to try and vectorize them
+    // again. Their distance will be "rebased" to use NewBaseInstIdx as
+    // reference.
+    for (auto [Dist, InstIdx] : PrevSet) {
+      if (InstIdx >= MinSafeIdx) {
+        insertOrLookup(InstIdx, Dist - DistFromCurBase);
+      }
+    }
+  }
+
+  /// Remove all stores that have been vectorized from this group.
+  void clearVectorizedStores(const BoUpSLP::ValueSet &VectorizedStores) {
+    const auto Begin = Instrs.begin();
+    auto NonVectorizedStore = Instrs.end();
+
+    while (NonVectorizedStore != Begin) {
----------------
gbossu wrote:

The code is now split into small functions with a clear name and purpose. I really don't believe it is harder to read.

Regarding iterators, there is unfortunately no nice way to convert backward to forward iterators in C++, I do not really want to use hacks like `--(It.base())`. `std::map::iterator` is bidirectional, it is designed to be able to go backwards, so using `std::prev()` feels acceptable to me.

That being said, I can change the code to use something like `copy_if` if you prefer. I would actually find this more readable, but given your previous comments about compile time, I assumed you would prefer the `std::map::erase` solution.

https://github.com/llvm/llvm-project/pull/134605


More information about the llvm-commits mailing list