[PATCH] D18366: Factor PrologEpilogInserter around spilling, frame finalization, and scavenging

Quentin Colombet via llvm-commits llvm-commits at lists.llvm.org
Fri May 13 09:56:34 PDT 2016


qcolombet added inline comments.

================
Comment at: lib/CodeGen/PrologEpilogInserter.cpp:87
@@ +86,3 @@
+    if (UsesCalleeSaves)
+      MFP.set(MachineFunctionProperties::Property::AllVRegsAllocated);
+    return MFP;
----------------
dschuff wrote:
> qcolombet wrote:
> > I do not like that UsesCalleeSaves implies all vregs are allocated. That is the same naming problem I mentioned previously, We need to come up with a name for the hook that conveys all that information at once. usesPhysReg for PEI?
> Do you mean `usesPhysRegForPEI`? or we could be more generic and do `usesVirtRegsOnly`?.
> 
> One issue with the code as-is, is that there's no way to get a `targetRegisterInfo` without a function (because it hangs off the subtarget, which can be different for each function). We could add a new method to TargetMachine itself but that seems like a big hammer. I'm not sure what else you could do though if you really want to do the pass configuration at instantiation time like this.
> Do you mean usesPhysRegForPEI? 

Yes, that’s what I meant, sorry.

I don’t think this should be in TargetRegisterInfo. I believe this property should hold for the target and not differ between sub targets, otherwise we are mixing some strange code :).
A hook in TargetMachine maybe?


http://reviews.llvm.org/D18366





More information about the llvm-commits mailing list