[llvm] 0d797b7 - RegisterCoaleser: Fix empty subrange verifier error

[Matt Arsenault via llvm-commits]

Author: Matt Arsenault
Date: 2023-07-24T12:18:34-04:00
New Revision: 0d797b71eb1012e76101e237b3d93aea6a5ba2be

URL: https://github.com/llvm/llvm-project/commit/0d797b71eb1012e76101e237b3d93aea6a5ba2be
DIFF: https://github.com/llvm/llvm-project/commit/0d797b71eb1012e76101e237b3d93aea6a5ba2be.diff

LOG: RegisterCoaleser: Fix empty subrange verifier error

In this example an implicit def had live-out undef subrange
defs. After coalescing with the def from a previous block, the
undef-defed lanes are no longer live out of the block in the new
interval. An empty subrange was tenatively created for these lanes,
but it must be deleted.

Added: 
    

Modified: 
    llvm/lib/CodeGen/RegisterCoalescer.cpp
    llvm/test/CodeGen/AMDGPU/blender-coalescer-verifier-error-empty-subrange.mir

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/RegisterCoalescer.cpp b/llvm/lib/CodeGen/RegisterCoalescer.cpp
index 0d56fa6dda48c8..e49885b6ad9677 100644
--- a/llvm/lib/CodeGen/RegisterCoalescer.cpp
+++ b/llvm/lib/CodeGen/RegisterCoalescer.cpp
@@ -1496,11 +1496,18 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
           LLVM_DEBUG(dbgs()
                      << "Removing undefined SubRange "
                      << PrintLaneMask(SR.LaneMask) << " : " << SR << "\n");
-          // VNI is in ValNo - remove any segments in this SubRange that have this ValNo
+
           if (VNInfo *RmValNo = SR.getVNInfoAt(CurrIdx.getRegSlot())) {
+            // VNI is in ValNo - remove any segments in this SubRange that have
+            // this ValNo
             SR.removeValNo(RmValNo);
-            UpdatedSubRanges = true;
           }
+
+          // We may not have a defined value at this point, but still need to
+          // clear out any empty subranges tentatively created by
+          // updateRegDefUses. The original subrange def may have only undefed
+          // some lanes.
+          UpdatedSubRanges = true;
         } else {
           // We know that this lane is defined by this instruction,
           // but at this point it may be empty because it is not used by

diff  --git a/llvm/test/CodeGen/AMDGPU/blender-coalescer-verifier-error-empty-subrange.mir b/llvm/test/CodeGen/AMDGPU/blender-coalescer-verifier-error-empty-subrange.mir
index 6046df605b7f95..3b1951e287d36a 100644
--- a/llvm/test/CodeGen/AMDGPU/blender-coalescer-verifier-error-empty-subrange.mir
+++ b/llvm/test/CodeGen/AMDGPU/blender-coalescer-verifier-error-empty-subrange.mir
@@ -1,17 +1,38 @@
-# RUN: not --crash llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1031 -run-pass=register-coalescer -verify-coalescing -o /dev/null %s 2>&1 | FileCheck %s
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 2
+# RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1031 -run-pass=register-coalescer -verify-coalescing -o - %s | FileCheck %s
 
 # Testcase variants from
 # liveout-implicit-def-subreg-redef-blender-verifier-error.mir which
 # hit other verifier errors after coalescing.
 
-# CHECK: *** Bad machine code: Subrange must not be empty ***
-
-
 # Same as previous, except the initial value isn't an implicit_def
 ---
 name: liveout_defined_register_redefine_sub0_implicit_def
 tracksRegLiveness: true
 body:             |
+  ; CHECK-LABEL: name: liveout_defined_register_redefine_sub0_implicit_def
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.2(0x40000000), %bb.1(0x40000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   S_CBRANCH_SCC0 %bb.2, implicit undef $scc
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.3(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   S_NOP 0, implicit-def %0
+  ; CHECK-NEXT:   undef %1.sub0:sgpr_128 = S_MOV_B32 0
+  ; CHECK-NEXT:   S_BRANCH %bb.3
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   successors: %bb.3(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   undef %0.sub0:sgpr_128 = IMPLICIT_DEF
+  ; CHECK-NEXT:   undef %1.sub0:sgpr_128 = IMPLICIT_DEF
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.3:
+  ; CHECK-NEXT:   S_NOP 0, implicit %0
+  ; CHECK-NEXT:   S_NOP 0, implicit %1.sub0
+  ; CHECK-NEXT:   S_ENDPGM 0
   bb.0:
     S_CBRANCH_SCC0 %bb.2, implicit undef $scc
 
@@ -30,3 +51,50 @@ body:             |
     S_ENDPGM 0
 
 ...
+
+# Compare with first
+---
+name: second_def_is_real
+tracksRegLiveness: true
+body:             |
+  ; CHECK-LABEL: name: second_def_is_real
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.2(0x40000000), %bb.1(0x40000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   S_CBRANCH_SCC0 %bb.2, implicit undef $scc
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.3(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   S_NOP 0, implicit-def %0
+  ; CHECK-NEXT:   [[S_MOV_B32_:%[0-9]+]]:sgpr_32 = S_MOV_B32 0
+  ; CHECK-NEXT:   S_BRANCH %bb.3
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   successors: %bb.3(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   undef %0.sub0:sgpr_128 = S_MOV_B32 123
+  ; CHECK-NEXT:   [[S_MOV_B32_1:%[0-9]+]]:sgpr_32 = S_MOV_B32 123
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.3:
+  ; CHECK-NEXT:   S_NOP 0, implicit %0
+  ; CHECK-NEXT:   S_NOP 0, implicit [[S_MOV_B32_1]]
+  ; CHECK-NEXT:   S_ENDPGM 0
+  bb.0:
+    S_CBRANCH_SCC0 %bb.2, implicit undef $scc
+
+  bb.1:
+    S_NOP 0, implicit-def %0:sgpr_128
+    %1:sgpr_32 = S_MOV_B32 0
+    S_BRANCH %bb.3
+
+  bb.2:
+    undef %0.sub0:sgpr_128 = S_MOV_B32 123
+    %1:sgpr_32 = COPY %0.sub0
+
+  bb.3:
+    S_NOP 0, implicit %0
+    S_NOP 0, implicit %1
+    S_ENDPGM 0
+
+...