[PATCH] D55301: RegAlloc: Allow targets to split register allocation

Quentin Colombet via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 12 10:05:23 PST 2019

qcolombet added a comment.
Herald added a subscriber: jdoerfert.

Hi Matt,

Couple of nitpicks inline.
My online remaining concern is exposing ClearVirtRegs.


Comment at: lib/CodeGen/RegAllocBase.cpp:179
+  if (!ShouldAllocateClass(*TRI, RC))
+    return;
For debugging purposes, add a DEBUG statement for each case.

Comment at: lib/CodeGen/RegAllocFast.cpp:73
-    RegAllocFast() : MachineFunctionPass(ID), StackSlotForVirtReg(-1) {}
+    RegAllocFast(RegClassFilterFunc F = allocateAllRegClasses,
+                 bool ClearVirtRegs_ = true) :
Should this be `const RegClassFilterFunc &` everywhere?

Comment at: lib/CodeGen/RegAllocFast.cpp:78
+      StackSlotForVirtReg(-1),
+      ClearVirtRegs(ClearVirtRegs_) {
+    }
It feels dangerous to expose the ClearVirtRegs to me.
Could we deduce what has to be cleared based on what we allocate instead of exposing this?

Comment at: lib/CodeGen/RegAllocFast.cpp:1350
+  std::function<bool(const TargetRegisterInfo &TRI,
+                     const TargetRegisterClass &RC)> Ftor, bool ClearVirtRegs) {
+  return new RegAllocFast(Ftor, ClearVirtRegs);
Could we have just one createFastRegisterAllocator with default arguments?
(Also ClearVirtReg should disappear per my other comment IMO).

Comment at: lib/CodeGen/RegAllocGreedy.cpp:603
Ditto: Just one createXXX method.



More information about the llvm-commits mailing list