[PATCH] D27293: [WIP] Cleanup SplitCSR implementation

Quentin Colombet via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 2 09:22:43 PST 2016


qcolombet added a comment.

> [speaking of getMinimanRCForPhysReg]  I don't think we have a better generic notion today,

I concur, we don't have a better generic notion.

> so all that is left is to enumerate register classes that allow suitable spilling and cheaply copying between the registers inside the class.

I would expect this to be a non-problem. The splitting mechanism should relax the constraints if need be. Therefore, yes, at first it will be overconstrained, but when split, it will not. The bottom line is that I am not sure the added enumeration is worth bothering, but this is a trade-off.
I don't mind either way.



================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:13381
     BuildMI(*Entry, MBBI, DebugLoc(), TII->get(TargetOpcode::COPY), NewVR)
-        .addReg(*I);
+      .addReg(*I)->dump();
 
----------------
Put the dump changes into a different patch.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:33766
+bool X86TargetLowering::supportSplitCSR(MachineFunction *MF) const {
+  // Why is this requirement here?
   if (!Subtarget.is64Bit())
----------------
I would rather you get to the bottom of this than having this comment.
Check with Manman.


https://reviews.llvm.org/D27293





More information about the llvm-commits mailing list