[PATCH] D127281: [Greedy RegAlloc] Fix the handling of split register in last chance re-coloring.

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet added inline comments.


================
Comment at: llvm/lib/CodeGen/RegAllocGreedy.cpp:1907
       std::tie(LI, PhysReg) = RecolorStack[I];
-      Matrix->assign(*LI, PhysReg);
+      if (!LI->empty() && !MRI->reg_nodbg_empty(LI->reg()))
+        Matrix->assign(*LI, PhysReg);
----------------
skatkov wrote:
> arsenm wrote:
> > The possibility for splitting during last chance recoloring confused me when I was last looking at this here. Is it a problem that the split was also not rolled back as well?
> Exactly, the problems comes from the fact that register we are trying to recolor is split in splirOrSelect and split always results in no assignment to physical register, as a result register is not recolored and we should do a roll back.
> 
> Current rollback implementation just returns back register we wanted to recolor to original physical register however this register is split and has no uses now.
> 
> In an example:
> We have register A to try last chance to color.
> We take the physical register P and find all registers assigned to P and interfere with live interval of A. Let it be only one register R.
> Then we unassign R from P and assign P to A. Fix the register P (do not use it in allocation) and try to recolor R.
> As a result of selectOrSplit R is split into R1, R2, R3. R has no uses in machine instructions now, its uses are replaced with R1, R2, R3 and corresponding copy instructions are inserted to connect R1, R2, R3.
> selectOrSplit always returns no assignment and R1, R2, R3 as new registers to be added again into queue and process them further on regular basis.
> As soon as we are in last chance color, we get no assignment of R to physical register and states this as a failure of recoloring.
> So we do a rollback. In this rollback we just return back an assignment of R to P. But R has no uses now and it leads to different assertions later when handling such registers (without uses but with assignment to physical registers).
> 
> This is a description of the problem.
> 
> Now how to solve this in rollback:
> 1) This patch just avoid assignment of P to R due to R has no uses. R1, R2, R3 will go to the queue finally and will processed on next iterations (it is actually change you did recently).
> 2) We can try to assign R1, R2, R3 to P. It has more complexity in implementation due to no easy way to detect that R is actually split into R1, R2, R3. So we need some preparation work.
> 3) We can reject split in selectOrSplit while we are doing last chance recoloring.
> 4) We can do even smarter: If we detect split R to R1, R2, R3 we can try to continue recolor R1, R2, R3 or only those Ri which still interfere with A.
> 
> May be there are other solutions. But just for bug fix I choose the less intrusive way. I hope it helps and not confuse :)
> 
> 
> 
The fact that last chance recoloring allows splitting is by design.
For split regs, we should leave them unassigned when the recoloring fails.


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

https://reviews.llvm.org/D127281