[PATCH] D95273: AMDGPU: Reduce the number of expensive calls in SIFormMemoryClause

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIFormMemoryClauses.cpp:332
 
-      RegUse Defs, Uses;
-      GCNDownwardRPTracker RPT(*LIS);
-      RPT.reset(MI);
+      if (!InitializedInBlock) {
+        RPT.reset(MI);
----------------
I am not sure, but likely RPT.getNext().isValid() has the same result as using !InitializedInBlock.


================
Comment at: llvm/lib/Target/AMDGPU/SIFormMemoryClauses.cpp:337
+        RPT.advance(MachineBasicBlock::const_iterator(MI));
+        RPT.advance();
+      }
----------------
This second advance is not needed. reset() sets iterator before MI, not after.


================
Comment at: llvm/lib/Target/AMDGPU/SIFormMemoryClauses.cpp:340
 
-      if (!processRegUses(MI, Defs, Uses, RPT))
+      auto LiveRegs = RPT.getLiveRegs();
+      RegUse Defs, Uses;
----------------
LiveRegSet LiveRegs = ...
Make sure you are actually copying it, not taking a reference.


================
Comment at: llvm/lib/Target/AMDGPU/SIFormMemoryClauses.cpp:363
+      // Restore the state after processing the bundle.
+      RPT.reset(MI, &LiveRegs);
+
----------------
Move it down after "Ind->insertMachineInstrInMaps(*B);" and reset on B, not on MI. Technically MI's iterator is not valid for RPT purposes after bundling.


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

https://reviews.llvm.org/D95273