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

[Serguei Katkov via Phabricator via llvm-commits]

skatkov created this revision.
skatkov added reviewers: arsenm, reames, MatzeB, craig.topper.
Herald added subscribers: kosarev, StephenFan, kerbowa, hiraditya, jvesely, qcolombet.
Herald added a project: All.
skatkov requested review of this revision.
Herald added a subscriber: wdng.
Herald added a project: LLVM.

This is a fix for https://github.com/llvm/llvm-project/issues/55827.

When register we are trying to re-color is split the original register (we tried to recover)
has no uses after the split. However in rollback actions we assign back physical register to it.
Later it causes different assertions. One of them is in attached test.

This CL fixes this by avoiding assigning physical register back to register which has no usage
or its live interval now is empty.

There are alternative solution:

1. Disable split in last chance re-coloring as soon as it always leads to rollback.
2. Replace register we are re-coloring with registers it was split to.
3. Move even further, (2) plus try to color split registers again.

This solution was chosen is less intrusive and fixes functional bug.


https://reviews.llvm.org/D127281

Files:
  llvm/lib/CodeGen/RegAllocGreedy.cpp
  llvm/test/CodeGen/AArch64/regalloc-last-chance-recolor-with-split.mir
  llvm/test/CodeGen/AMDGPU/remaining-virtual-register-operands.ll


Index: llvm/test/CodeGen/AMDGPU/remaining-virtual-register-operands.ll
===================================================================
--- llvm/test/CodeGen/AMDGPU/remaining-virtual-register-operands.ll
+++ llvm/test/CodeGen/AMDGPU/remaining-virtual-register-operands.ll
@@ -1,32 +1,17 @@
-; RUN: not llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx908 -verify-machineinstrs < %s 2>%t.err | FileCheck %s
-; RUN: FileCheck -check-prefix=ERR %s < %t.err
+; RUN: not --crash llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx908 -verify-machineinstrs < %s 2>&1 | FileCheck %s
 
 ; This testcase fails register allocation at the same time it performs
 ; virtual register splitting (by introducing VGPR to AGPR copies). We
 ; still need to enqueue and allocate the newly split vregs after the
 ; failure.
 
+; The machine verifier complains about usage of register
+; which is marked as killed in previous instruction.
+; This happens due to when register allocator is out of registers
+; it takes the first avialable register.
 
-; ERR: error: ran out of registers during register allocation
-; ERR-NEXT: error: ran out of registers during register allocation
-; ERR-NEXT: error: ran out of registers during register allocation
-; ERR-NOT: ERROR
-
-; CHECK: v_accvgpr_write_b32
-; CHECK: v_accvgpr_write_b32
-; CHECK: v_accvgpr_write_b32
-; CHECK: v_accvgpr_write_b32
-; CHECK: v_accvgpr_write_b32
-; CHECK: v_accvgpr_write_b32
-; CHECK: v_accvgpr_write_b32
-
-; CHECK: v_accvgpr_read_b32
-; CHECK: v_accvgpr_read_b32
-; CHECK: v_accvgpr_read_b32
-; CHECK: v_accvgpr_read_b32
-; CHECK: v_accvgpr_read_b32
-; CHECK: v_accvgpr_read_b32
-; CHECK: v_accvgpr_read_b32
+; CHECK: error: ran out of registers during register allocation
+; CHECK: Bad machine code: Using an undefined physical register
 define amdgpu_kernel void @alloc_failure_with_split_vregs(float %v0, float %v1) #0 {
   %agpr0 = call float asm sideeffect "; def $0", "=${a0}"()
   %agpr.vec = insertelement <16 x float> undef, float %agpr0, i32 0
Index: llvm/test/CodeGen/AArch64/regalloc-last-chance-recolor-with-split.mir
===================================================================
--- llvm/test/CodeGen/AArch64/regalloc-last-chance-recolor-with-split.mir
+++ llvm/test/CodeGen/AArch64/regalloc-last-chance-recolor-with-split.mir
@@ -1,6 +1,6 @@
-# RUN: not --crash llc -mattr="+reserve-x28" --start-before=greedy -verify-machineinstrs %s -o - 2>&1 | FileCheck %s
+# RUN: llc -mattr="+reserve-x28" --start-before=greedy -verify-machineinstrs %s -o - 2>&1 | FileCheck %s
 
-# CHECK: Non-empty but used interval
+# CHECK-LABEL: ham
 
 --- |
   target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
Index: llvm/lib/CodeGen/RegAllocGreedy.cpp
===================================================================
--- llvm/lib/CodeGen/RegAllocGreedy.cpp
+++ llvm/lib/CodeGen/RegAllocGreedy.cpp
@@ -1904,7 +1904,8 @@
       const LiveInterval *LI;
       MCRegister PhysReg;
       std::tie(LI, PhysReg) = RecolorStack[I];
-      Matrix->assign(*LI, PhysReg);
+      if (!LI->empty() && !MRI->reg_nodbg_empty(LI->reg()))
+        Matrix->assign(*LI, PhysReg);
     }
 
     // Pop the stack of recoloring attempts.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D127281.435087.patch
Type: text/x-patch
Size: 3181 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220608/06e4b481/attachment.bin>