[PATCH] D157719: [AMDGPU] [SIFrameLowering] Replace LivePhysRegs with LiveRegUnits

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/LiveRegUnits.h:115-120
+  /// Returns true if no part of non-reserved physical register \p Reg is live
+  /// (overloaded)
+  bool available(const MachineRegisterInfo &MRI, MCPhysReg Reg) const;
+
+  /// Returns true if no part of physical register \p Reg is live. (overloaded)
+  bool available(MCPhysReg Reg) const;
----------------
This is potentially confusing, I think either they need to have separate names indicating one considers reserved registers or not. Or there should just be the one that does consider reserved registers. Can't think of a case off the top of my head where you would ever want to ignore reservations


================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:34-37
   for (MCRegister Reg : RC) {
-    if (!MRI.isPhysRegUsed(Reg) && LiveRegs.available(MRI, Reg))
+    if (!MRI.isPhysRegUsed(Reg) && LiveUnits.available(MRI, Reg))
       return Reg;
   }
----------------
The isPhysRegUsed check should be redundant

This loop seems backwards, we could start by finding unused regunits and go up to the the register


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157719