[PATCH] D139732: [AMDGPU] Add pass to rewrite partially used virtual superregisters after RenameIndependentSubregs pass with registers of minimal size.

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/GCNRewritePartialRegUses.cpp:75
+  // right and replacing the original register with a register of minimal size.
+  // Return true the change has been made.
+  bool rewriteReg(Register Reg) const;
----------------
missing 'if'. Also should /// for this and the other comments


================
Comment at: llvm/lib/Target/AMDGPU/GCNRewritePartialRegUses.cpp:154-155
+
+unsigned GCNRewritePartialRegUses::getSubReg(unsigned Offset,
+                                             unsigned Size) const {
+  auto R = SubRegs.try_emplace({Offset, Size}, 0);
----------------
Feels like tablegen should generate something like this which does a binary search of subregister indices


================
Comment at: llvm/lib/Target/AMDGPU/GCNRewritePartialRegUses.cpp:215-217
+  for (auto &P : SubRegs) {
+    unsigned OldSubReg = P.first;
+    unsigned &NewSubReg = P.second.SubReg;
----------------
can you do auto &[OldSubReg, NewSubReg] : SubRegs)?


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.h:423-426
+    assert(AlignNumBits != 0);
+    unsigned RCAlign = getRegClassAlignmentNumBits(RC);
+    return RCAlign == AlignNumBits ||
+           (RCAlign > AlignNumBits && (RCAlign % AlignNumBits) == 0);
----------------
This should be implied by subregister support. Can you check getSubClassWithSubReg/getMatchingSuperRegClass?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139732