[PATCH] D85517: [Scheduling] Implement a new way to cluster loads/stores

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineScheduler.cpp:1533
 protected:
-  void clusterNeighboringMemOps(ArrayRef<SUnit *> MemOps, ScheduleDAGInstrs *DAG);
+  void clusterNeighboringMemOps(SmallVector<MemOpInfo, 32> &MemOps,
+                                ScheduleDAGInstrs *DAG);
----------------
I think this can still be an `ArrayRef`.


================
Comment at: llvm/lib/CodeGen/MachineScheduler.cpp:1536
+  void collectMemOpRecords(std::vector<SUnit> &SUnits,
+                           SmallVector<MemOpInfo, 32> &MemOpRecords);
 };
----------------
I think this should take a `SmallVectorImpl<MemOpInfo>&`.


================
Comment at: llvm/lib/CodeGen/MachineScheduler.cpp:1607
+    unsigned ClusterLength = 2;
+    unsigned CurrentClusterBytes = MemOpb.Width;
+    if (SUnit2ClusterInfo.count(MemOpa.SU->NodeNum)) {
----------------
Shouldn't this be `MemOpa.Width+MemOpb.Width`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85517/new/

https://reviews.llvm.org/D85517