[PATCH] D70379: [AMDGPU] Reserving VGPR for future SGPR Spill

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:100
+    "amdgpu-reserve-vgpr-for-sgpr-spill",
+    cl::desc("Allocates one VGPR for future SGPR Spill"), cl::init(false));
+
----------------
This should be the default, but the test disruption is probably significant without forcing the VGPR to be the high register, and adjust it down after RA


================
Comment at: llvm/test/CodeGen/AMDGPU/reserve-vgpr-for-sgpr-spill.ll:12
+; GCN: ; NumVgprs: 256
+define fastcc i32 @parent_func(i32 %0, i32 %1, [255 x i32] %b) #1 {
+entry:
----------------
I don't see how this test really stresses the need for SGPR spilling with no free VGPRs. I would expect this to look more like the tests in spill-wide-sgpr.ll, or spill-scavenge-offset.ll to force a high pressure


================
Comment at: llvm/test/CodeGen/AMDGPU/reserve-vgpr-for-sgpr-spill.ll:20
+attributes #0 = { nounwind }
+attributes #1 = { nounwind "no-frame-pointer-elim"="true" }
----------------
This is the removed form of the attribute. This should be something like frame-pointer=none, although I don't think it matters here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70379