[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 a comment.

I think we need to have LiveRangeEdit start doing the same thing. I guess we can start with this in a separate pass for now, and then I can look into merge it into LiveRangeEdit. One of the problems I'm trying to solve is tuple spills increasing liveness for dead lanes



================
Comment at: llvm/lib/Target/AMDGPU/GCNRewritePartialRegUses.cpp:81
+    /// Register class required to hold the value stored in the SubReg.
+    const TargetRegisterClass *RC;
+
----------------
Should use a const reference, this should never need to be null 


================
Comment at: llvm/lib/Target/AMDGPU/GCNRewritePartialRegUses.cpp:91
+  /// Map OldSubReg -> { RC, NewSubReg }. Used as in/out container.
+  typedef SmallDenseMap<unsigned, SubRegInfo> SubRegMap;
+
----------------
Shouldn't this be a static table?


================
Comment at: llvm/lib/Target/AMDGPU/GCNRewritePartialRegUses.cpp:154
+
+unsigned GCNRewritePartialRegUses::getSubReg(unsigned Offset,
+                                             unsigned Size) const {
----------------
Add TODO to move to tablegen 


================
Comment at: llvm/lib/Target/AMDGPU/GCNRewritePartialRegUses.cpp:220
+
+    // Instruction operand may not specify required register class (ex. COPY).
+    if (!SubRegRC)
----------------
We have helpers to query the class from the instruction, this shouldn't be reachable?


================
Comment at: llvm/lib/Target/AMDGPU/GCNRewritePartialRegUses.cpp:264
+    unsigned NumBits = TRI->getRegSizeInBits(*RC);
+#ifdef DUMP_ALL_SUPERREG_CLASSES
+    LLVM_DEBUG(dbgs() << "  " << NumBits << ' ' << TRI->getRegClassName(RC)
----------------
For all practical purposes this is dead code, either just enable with with all debug printing or remove it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139732