[PATCH] D139673: [AMDGPU][SIFrameLowering] Mark VGPR used for AGPR spills as reserved

[Matt Arsenault via Phabricator via llvm-commits]

arsenm accepted this revision.
arsenm added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/include/llvm/CodeGen/MachineRegisterInfo.h:916-921
+  void unreserveReg(MCRegister PhysReg, const TargetRegisterInfo *TRI) {
+    MCRegAliasIterator R(PhysReg, TRI, true);
+
+    for (; R.isValid(); ++R)
+      ReservedRegs.reset(*R);
+  }
----------------
I think blindly unreserving all aliases doesn't make as much sense as it does in the reserve direction. You're also not using this version, so might as well drop it for now.


================
Comment at: llvm/include/llvm/CodeGen/MachineRegisterInfo.h:906
+  void reserveReg(MCRegister PhysReg) {
+    ReservedRegs.set(PhysReg);
+  }
----------------
jrbyrnes wrote:
> arsenm wrote:
> > This needs to consider all aliasing registers. This also needs a warning that this cannot be used in the middle of a walk in a function or when you have liveness info available
> Hey Matt -- thanks for comments thus far. Regarding the warning -- were you thinking programmatically enforcing these conditions (e.g. via assert) or a literal warning in comment? Not sure how we can enforce this state with MRI
I don't know how you could programmatically enforce this. Should also assert the reserved regs are already frozen before; this should only be used for incremental updates


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139673