[PATCH] D132436: [AMDGPU][SIFrameLowering] Unify custom SGPR spill saves and restores (NFC)

[Sebastian Neubauer via Phabricator via llvm-commits]

sebastian-ne added a comment.

Looks ok to me in general.



================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:31
+// throughout the function. On failure, returns a null register.
+static MCRegister findUnusedScratchNonCSR(MachineRegisterInfo &MRI,
+                                          LivePhysRegs &LiveRegs,
----------------
This function doesn’t do anything with CSRs, although the name says so?


================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:100
     } else {
       // Remove dead <NewFI> index
+      MF.getFrameInfo().RemoveStackObject(FI);
----------------
Comment should say <FI> for the renamed variable.


================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:269
+
+  void copyToScratchSGPR(Register DstReg) const {
+    const TargetRegisterClass *RC = TRI.getPhysRegClass(DstReg);
----------------
Why does this code split the copies into SubRegs and copyFromScratchSGPR doesn’t?


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.h:37-38
+// at function prolog/epilog.
+namespace RegSaveInfo {
+enum RegSaveKind : uint8_t {
+  COPY_TO_SCRATCH_SGPR,
----------------
Why not make this an `enum class RegSaveKind`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132436