[PATCH] D16481: Split PrologEpilogInserter into 2 parts

Derek Schuff via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 4 13:43:43 PST 2016


dschuff added inline comments.

================
Comment at: lib/CodeGen/PrologEpilogInserter.cpp:264
@@ +263,3 @@
+
+bool PEIWithCSRs::runOnMachineFunction(MachineFunction &Fn) {
+  const TargetFrameLowering *TFI = Fn.getSubtarget().getFrameLowering();
----------------
hfinkel wrote:
> Did we decide that we need to split the passes this way? It seems like the boilerplate from having two passes seems >= the difference between this runOnMachineFunction implementation and the simpler version.
The motivation for this change was the really strong desire expressed to have clear separation at the pass level between passes that support virtual registers and passes that do not. (see also http://reviews.llvm.org/D16483). If we implement that, then we have to have a PEI pass that supports virtual registers.

Of course we could just declare that this PEI pass supports virtual registers. From my own experience and auditing of the code, it seems to work fine for WebAssembly. The subsets that WebAssembly uses (i.e. the parts that I've broken out here into the base PEI) are the only parts that actually run when the target returns an empty set of CSRs; I've basically just made that distinction explicit in this change. I'd be interested in getting an opinion from @qcolombet based on this more concrete change.

================
Comment at: lib/CodeGen/PrologEpilogInserter.cpp:356
@@ -295,3 +355,3 @@
   // These are used to keep track the callee-save area. Initialize them.
-  MinCSFrameIndex = INT_MAX;
+  MinCSFrameIndex = std::numeric_limits<unsigned>::max();
   MaxCSFrameIndex = 0;
----------------
hfinkel wrote:
> Does having > INT_MAX FIs really work with just these changes?
No, this change was just to make the types (signed vs unsigned) consistent (see also line 664 below), and, if there's going to be a sentinel value, it might as well be the max for the actual type used (unsigned) instead of a different type.


http://reviews.llvm.org/D16481





More information about the llvm-commits mailing list