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

Mon Jun 26 21:23:28 PDT 2023

yassingh added inline comments.

================
Comment at: llvm/lib/Target/AMDGPU/SILowerSGPRSpills.cpp:66
+    return MachineFunctionProperties()
+        .set(MachineFunctionProperties::Property::IsSSA)
+        .set(MachineFunctionProperties::Property::NoVRegs);
----------------
cdevadas wrote:
> yassingh wrote:
> > yassingh wrote:
> > > 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 ?
> > Removing this line works fine when running the whole pipeline as the compiler knows the code here is not in SSA form. However, when SILowerSGPRSpills and related passes are run in isolation the verifier assumes the code to be in SSA form(possibly a bug there, also we are introducing virtual vgprs maybe that's the reason). 
> > 
> > I can leave the line as it is or is there some way to update the test files to let the compiler know the input isn't SSA? I tried "isSSA: false", didn't work.
> Seems reasonable to retain this line for now. The compiler might not be able to decide that this pass is run post phi-elimination and assume SSA form by default. There must be a serialized option to control it for MIR tests.
Yeah, MIRParser::isSSA recomputes the SSA information and sets it to true, also it doesn't expose a way to override it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124196