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

[Quentin Colombet via llvm-commits]

qcolombet added a comment.

Hi Derek,

Thanks for keep pushing on that.

The HasVirtualRegister will not change for a given target, thus, I would rather have it checked only at the instantiation of the class.
In other words, instead of having two if blocks in runOnMachineFunction, I would suggest calling a helper object that would have been instantiated differently according to the mode.

E.g., instead of doing
for each function
if (HasVirtual) {

  // handlingA

} else {

  // handlingAbis

}

I would do:
Once for the Module:
if (HasVirtual) {
 helper = helperA
} else {
 helper = helperAbis // where helperAbis is a sub class of helperA and helperA does not nothing by default.
}

for each function

  helper->callA

What do you think?

Cheers,
-Quentin


================
Comment at: lib/CodeGen/PrologEpilogInserter.cpp:148
@@ +147,3 @@
+  // not require register scavenging.
+  const bool HasVirtualRegisters = Fn.getRegInfo().getNumVirtRegs();
+
----------------
HasVirtualRegisters should be an argument of the pass and we should have an assert HasVirtualRegisters or ! Fn.getRegInfo().getNumVirtRegs().
The rationale is that some backends might end up having virtual registers here whereas they should not.


http://reviews.llvm.org/D18366