[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:196
+  if (Inserted) {
+    auto &V = I->second;
+    V.resize(TRI->getNumRegClasses());
----------------
I have no idea what V is, use an explicit type?


================
Comment at: llvm/lib/Target/AMDGPU/GCNRewritePartialRegUses.cpp:257
+  // allocatable, aligned, has all shifted subregs and each subreg has required
+  // register class (see SubRegRC above). Now select first (that is largest)
+  // register class with registers of minimal size.
----------------
I thought the classes were already supposed to be sorted with largest first such that you only need to look at the first set bit?


================
Comment at: llvm/lib/Target/AMDGPU/GCNRewritePartialRegUses.cpp:288
+                                        SubRegMap &SubRegs) const {
+  unsigned CoverSubreg = 0;
+  unsigned Offset = std::numeric_limits<unsigned>::max();
----------------
AMDGPU::NoSubRegister?


================
Comment at: llvm/lib/Target/AMDGPU/GCNRewritePartialRegUses.cpp:391
+    NewLI.assign(OldLI, Allocator);
+  NewLI.verify(MRI);
+  LIS->removeInterval(OldReg);
----------------
I don't think verify is defined in a release build


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139732