[PATCH] D38597: [PEI] Remove required properties and use 'if' instead of std::function

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added inline comments.


================
Comment at: llvm/lib/CodeGen/PrologEpilogInserter.cpp:155
   const TargetFrameLowering *TFI = Fn.getSubtarget().getFrameLowering();
+  bool UsesCalleeSaves = Fn.getTarget().usesPhysRegsForPEI();
 
----------------
I'd name the variable `UsesPhysRegsForPEI`.


================
Comment at: llvm/lib/CodeGen/PrologEpilogInserter.cpp:174
+  if (UsesCalleeSaves)
+    doSpillCalleeSavedRegs(Fn, RS, MinCSFrameIndex, MaxCSFrameIndex, SaveBlocks,
+                           RestoreBlocks);
----------------
With the std::function gone we can rename `doSpillCalleeSavedRegs` back to `spillCalleeSavedRegs`


================
Comment at: llvm/lib/CodeGen/PrologEpilogInserter.cpp:201-205
+    if (UsesCalleeSaves)
+      scavengeFrameVirtualRegs(Fn, *RS);
 
-      // Clear any vregs created by virtual scavenging.
-      Fn.getRegInfo().clearVirtRegs();
+    // Clear any vregs created by virtual scavenging.
+    Fn.getRegInfo().clearVirtRegs();
----------------
This feels wrong, we should only `clearVirtRegs()` when we actually scavenged them. Checking the code it seems that the clearing is already done in `scavengeFrameVirtualRegs()` nowadays so we can remove it here. Maybe we should also skip the whole `if (UsesCalleeSaves)` then too as it seems target can already control this bit via TRI::requiresRegisterScavenging`.


https://reviews.llvm.org/D38597