[PATCH] D23814: AMDGPU/SI: Improve SILoadStoreOptimizer and run it before the scheduler

[Matt Arsenault via llvm-commits]

arsenm added inline comments.

================
Comment at: lib/Target/AMDGPU/SILoadStoreOptimizer.cpp:107
@@ -102,6 +106,3 @@
     AU.setPreservesCFG();
-    AU.addPreserved<SlotIndexes>();
-    AU.addPreserved<LiveIntervals>();
-    AU.addPreserved<LiveVariables>();
-    AU.addRequired<LiveIntervals>();
+    AU.addRequired<AAResultsWrapperPass>();
 
----------------
I think when I was looking at the hook it was for AA for scheduling

================
Comment at: lib/Target/AMDGPU/SILoadStoreOptimizer.cpp:141-143
@@ +140,5 @@
+                          SmallVectorImpl<const MachineOperand *> &Defs) {
+  for (const MachineOperand &Def : MI.defs()) {
+    Defs.push_back(&Def);
+  }
+}
----------------
I think you can directly initialize the SmallVector with the range, SmallVector<> Foo(MI.defs())

================
Comment at: lib/Target/AMDGPU/SILoadStoreOptimizer.cpp:149
@@ +148,3 @@
+                        ArrayRef<MachineInstr*> InstsToMove,
+                        const TargetInstrInfo *TII,
+                        AliasAnalysis *AA) {
----------------
SIInstrInfo

================
Comment at: lib/Target/AMDGPU/SILoadStoreOptimizer.cpp:155
@@ +154,3 @@
+  for (MachineInstr *InstToMove : InstsToMove) {
+    if (!InstToMove->mayLoad() && !InstToMove->mayStore())
+      continue;
----------------
Can use ->mayLoadOrStore

================
Comment at: lib/Target/AMDGPU/SILoadStoreOptimizer.cpp:216
@@ +215,3 @@
+
+      if ((MBBI->mayLoad() || MBBI->mayStore()) &&
+          !TII->areMemAccessesTriviallyDisjoint(*I, *MBBI, AA)) {
----------------
ditto

================
Comment at: lib/Target/AMDGPU/SILoadStoreOptimizer.cpp:364
@@ -298,1 +363,3 @@
+  unsigned EltSize,
+  ArrayRef<MachineInstr*>InstsToMove) {
   MachineBasicBlock *MBB = I->getParent();
----------------
Missing space

================
Comment at: lib/Target/AMDGPU/SILoadStoreOptimizer.cpp:442
@@ -391,2 +441,3 @@
 
+    std::vector<MachineInstr*> InstsToMove;
     unsigned Opc = MI.getOpcode();
----------------
SmallVector


https://reviews.llvm.org/D23814