[PATCH] D124196: [AMDGPU][SILowerSGPRSpills] Spill SGPRs to virtual VGPRs

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SILowerSGPRSpills.cpp:66
+    return MachineFunctionProperties()
+        .set(MachineFunctionProperties::Property::IsSSA)
+        .set(MachineFunctionProperties::Property::NoVRegs);
----------------
It shouldn't have been SSA to begin with ad this doesn't de-SSA


================
Comment at: llvm/lib/Target/AMDGPU/SILowerSGPRSpills.cpp:67
+        .set(MachineFunctionProperties::Property::IsSSA)
+        .set(MachineFunctionProperties::Property::NoVRegs);
+  }
----------------
Add a comment explaining the new vregs?


================
Comment at: llvm/lib/Target/AMDGPU/SILowerSGPRSpills.cpp:366
+          BuildMI(*InsertPt->getParent(), *InsertPt, InsertPt->getDebugLoc(),
+                  TII->get(AMDGPU::IMPLICIT_DEF), Reg);
+      if (LIS) {
----------------
cdevadas wrote:
> arsenm wrote:
> > It might be worth adding a target comment flag for this implicit def to comment it's for SGPR spilling
> I don't think we can do any special handling for them at assembly emission.
> IMPLICIT_DEF is handled/printed by the generic part of AsmPrinter and it won't reach the target-specific emitInstruction at all.
You don't need to specially handle the instruction, see AsmPrinterFlags


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124196