[PATCH] D117562: [AMDGPU] Sink immediate VGPR defs if high RP

[Valery Pykhtin via Phabricator via llvm-commits]

vpykhtin added a comment.

more comments to follow



================
Comment at: llvm/lib/Target/AMDGPU/GCNPreRAOptimizations.cpp:139
+        SeenSubRegDefs[Op.getSubReg()] = 1;
+    }
     switch (I.getOpcode()) {
----------------
you can use single ++SeenSubRegDefs[Op.getSubReg()] instead of all this "find and if/else". If there is no entry in the map it will be inserted with 0 value and then incremented.


================
Comment at: llvm/lib/Target/AMDGPU/GCNPreRAOptimizations.cpp:226
+
+      for (MachineInstr &UseI : MRI->use_instructions(Reg)) {
+        // First find if a use is within a loop
----------------
given that you've checked !MRI->hasOneUse(Reg) before you just need to take the first use, no need to iterate.


================
Comment at: llvm/lib/Target/AMDGPU/GCNPreRAOptimizations.cpp:241
+
+        if (VGPRImmDef || VGPRImmUse)
+          return false;
----------------
how VGPRImmUse can be true if you expect MRI->hasOneUse(Reg) ?


================
Comment at: llvm/lib/Target/AMDGPU/GCNPreRAOptimizations.cpp:268
+  // For VGPR reg, check if we can sink a imm def to a use
+  else if (TRI->isVGPRClass(RC) && (!VGPRImmDef || !VGPRImmUse))
     return Changed;
----------------
use IsVGPRDst instead of TRI->isVGPRClass(RC)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117562