[PATCH] D156491: [RA] Split a virtual register in cold blocks if it is not assigned preferred physical register

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet added inline comments.


================
Comment at: llvm/lib/CodeGen/RegAllocGreedy.cpp:2404
+  if (!NewVRegs.empty())
+    return 0;
 
----------------
Carrot wrote:
> qcolombet wrote:
> > qcolombet wrote:
> > > Splitting decisions are supposed to be taken later (`trySplit`), this breaks the general algorithm's flow.
> > Concretely, I'm asking if you could do this kind of splitting in the splitting heuristic.
> > You may want to add a new stage in the algorithm (and that'll come at a compile time cost.)
> The problem is we need to decide between a physical register and split, if we move it to trySplit, we will miss the possibility of assigning a physical register to the whole virtual register. trySplit compares splits against different physical registers, we need to compare split against non-split.
> Function tryAssignCSRFirstTime does similar thing. trySplitAroundHintReg is even more special because it tries to split against only 1 physical register.
> 
> Function tryAssignCSRFirstTime does similar thing.

True but I feel it is justified because these physreg are not free to use the first time.
I don't see this argument applying here.

> if we move it to trySplit, we will miss the possibility of assigning a physical register to the whole virtual register.

Hmm, I don't get that part, we're going to split in `trySplitAroundHintReg`, how do we get to assign the same physreg to the whole vreg?

>  we need to compare split against non-split.

I don't get that part either.
For a given vreg, the first time we get through trySplit, we compare non-split against split.

I'm not totally opposed to the patch, but I still fail to see why it doesn't fit in the regular splitting mechanism and hence why we need a special mechanism for this.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156491/new/

https://reviews.llvm.org/D156491