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

[Yashwant Singh via Phabricator via llvm-commits]

yassingh added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SILowerSGPRSpills.cpp:66
+    return MachineFunctionProperties()
+        .set(MachineFunctionProperties::Property::IsSSA)
+        .set(MachineFunctionProperties::Property::NoVRegs);
----------------
arsenm wrote:
> It shouldn't have been SSA to begin with ad this doesn't de-SSA
Removing this line causes machine-verifier to crash in few tests. Any hints @cdevadas ?


================
Comment at: llvm/lib/Target/AMDGPU/SILowerSGPRSpills.cpp:366
+          BuildMI(*InsertPt->getParent(), *InsertPt, InsertPt->getDebugLoc(),
+                  TII->get(AMDGPU::IMPLICIT_DEF), Reg);
+      if (LIS) {
----------------
arsenm wrote:
> 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
Tried adding a new flag here D153754


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124196