[RegisterCoalescer] Fix crash on early clobbered subreg operands.

[dfukalov]

Summary

The issue was with processing two subregs of the same reg are used in the same
instruction (e.g. inline asm): "def early-clobber" and other just "def".
Register coalescer ran in bad recursion if the early clobbered subreg is second
in the following sequence of COPYs.

[arsenm]

llvm/lib/CodeGen/RegisterCoalescer.cpp

2746–2749

Could use a comment

llvm/test/CodeGen/AMDGPU/coalescer-early-clobber-subreg.mir

8

Missing word between "and other"?

[dfukalov]

Addressed comments.

[dfukalov]

Beautified test.

llvm/lib/CodeGen/RegisterCoalescer.cpp

if (VNInfo *OtherVNI = OtherLRQ.valueDefined()) {
     else if (VNI->def < OtherVNI->def && OtherLRQ.valueIn()) {
       // This is an early-clobber def overlapping a live-in value in the other
       // register. Not mergeable.
       V.OtherVNI = OtherLRQ.valueIn();
       return CR_Impossible;
     }
     V.OtherVNI = OtherVNI;
     Val &OtherV = Other.Vals[OtherVNI->id];
-    // Keep this value, check for conflicts when analyzing OtherVNI.
-    if (!OtherV.isAnalyzed())
+    // Keep this value, check for conflicts when analyzing OtherVNI. Avoid
+    // revisiting OtherVNI->id in JoinVals::computeAssignment() below before it
+    // is assigned.
+    if (!OtherV.isAnalyzed() || Other.Assignments[OtherVNI->id] == -1)
       return CR_Keep;
     // Both sides have been analyzed now.
     // Allow overlapping PHI values. Any real interference would show up in a
     // predecessor, the PHI itself can't introduce any conflicts.
     if (VNI->isPHIDef())
       return CR_Merge;
     if ((V.ValidLanes & OtherV.ValidLanes).any())
       // Overlapping lanes can't be resolved.

llvm/test/CodeGen/AMDGPU/coalescer-early-clobber-subreg.mir

+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -march=amdgcn -verify-machineinstrs -run-pass=simple-register-coalescing -o - %s | FileCheck %s
+
+# Test used to crash with message:
+# JoinVals::computeAssignment(unsigned int, (anonymous namespace)::JoinVals &): Assertion `Assignments[ValNo] != -1 && "Bad recursion?"' failed.
+
+# The issue was with processing two operands are parts of the same reg and are
+# used in the same instruction (e.g. inline asm): first is "def early-clobber",
+# while the other is just "def".
+# Register coalescer ran in bad recursion if the early clobbered subreg is
+# second in the following sequence of COPYs.
+
+---
+name:            foo1
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $vgpr0_vgpr1
+
+    ; CHECK-LABEL: name: foo1
+    ; CHECK: liveins: $vgpr0_vgpr1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: INLINEASM &"", 0 /* attdialect */, 1835018 /* regdef:VGPR_32 */, def undef %2.sub0, 1835019 /* regdef-ec:VGPR_32 */, def undef early-clobber %2.sub1
+    ; CHECK-NEXT: FLAT_STORE_DWORDX2 $vgpr0_vgpr1, %2, 0, 0, implicit $exec, implicit $flat_scr :: (store (s64))
+    ; CHECK-NEXT: S_ENDPGM 0
+    INLINEASM &"", 0 /* attdialect */, 1835018 /* regdef:VGPR_32 */, def %0:vgpr_32, 1835019 /* regdef-ec:VGPR_32 */, def early-clobber %1:vgpr_32
+    undef %2.sub0:vreg_64 = COPY killed %0
+    %2.sub1:vreg_64 = COPY killed %1
+    FLAT_STORE_DWORDX2 killed $vgpr0_vgpr1, killed %2, 0, 0, implicit $exec, implicit $flat_scr :: (store (s64))
+    S_ENDPGM 0
+
+...
+
+---
+name:            foo2
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $vgpr0_vgpr1
+
+    ; CHECK-LABEL: name: foo2
+    ; CHECK: liveins: $vgpr0_vgpr1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: INLINEASM &"", 0 /* attdialect */, 1835019 /* regdef-ec:VGPR_32 */, def undef early-clobber %2.sub1, 1835018 /* regdef:VGPR_32 */, def undef %2.sub0
+    ; CHECK-NEXT: FLAT_STORE_DWORDX2 $vgpr0_vgpr1, %2, 0, 0, implicit $exec, implicit $flat_scr :: (store (s64))
+    ; CHECK-NEXT: S_ENDPGM 0
+    INLINEASM &"", 0 /* attdialect */, 1835019 /* regdef-ec:VGPR_32 */, def early-clobber %1:vgpr_32, 1835018 /* regdef:VGPR_32 */, def %0:vgpr_32
+    undef %2.sub0:vreg_64 = COPY killed %0
+    %2.sub1:vreg_64 = COPY killed %1
+    FLAT_STORE_DWORDX2 killed $vgpr0_vgpr1, killed %2, 0, 0, implicit $exec, implicit $flat_scr :: (store (s64))
+    S_ENDPGM 0
+
+...
+
+---
+name:            foo3
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $vgpr0_vgpr1
+
+    ; CHECK-LABEL: name: foo3
+    ; CHECK: liveins: $vgpr0_vgpr1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: INLINEASM &"", 0 /* attdialect */, 1835018 /* regdef:VGPR_32 */, def undef %2.sub0, 1835019 /* regdef-ec:VGPR_32 */, def undef early-clobber %2.sub1
+    ; CHECK-NEXT: FLAT_STORE_DWORDX2 $vgpr0_vgpr1, %2, 0, 0, implicit $exec, implicit $flat_scr :: (store (s64))
+    ; CHECK-NEXT: S_ENDPGM 0
+    INLINEASM &"", 0 /* attdialect */, 1835018 /* regdef:VGPR_32 */, def %1:vgpr_32, 1835019 /* regdef-ec:VGPR_32 */, def early-clobber %0:vgpr_32
+    undef %2.sub0:vreg_64 = COPY killed %1
+    %2.sub1:vreg_64 = COPY killed %0
+    FLAT_STORE_DWORDX2 killed $vgpr0_vgpr1, killed %2, 0, 0, implicit $exec, implicit $flat_scr :: (store (s64))
+    S_ENDPGM 0
+
+...
+
+---
+name:            foo4
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $vgpr0_vgpr1
+
+    ; CHECK-LABEL: name: foo4
+    ; CHECK: liveins: $vgpr0_vgpr1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: INLINEASM &"", 0 /* attdialect */, 1835019 /* regdef-ec:VGPR_32 */, def undef early-clobber %2.sub1, 1835018 /* regdef:VGPR_32 */, def undef %2.sub0
+    ; CHECK-NEXT: FLAT_STORE_DWORDX2 $vgpr0_vgpr1, %2, 0, 0, implicit $exec, implicit $flat_scr :: (store (s64))
+    ; CHECK-NEXT: S_ENDPGM 0
+    INLINEASM &"", 0 /* attdialect */, 1835019 /* regdef-ec:VGPR_32 */, def early-clobber %0:vgpr_32, 1835018 /* regdef:VGPR_32 */, def %1:vgpr_32
+    undef %2.sub0:vreg_64 = COPY killed %1
+    %2.sub1:vreg_64 = COPY killed %0
+    FLAT_STORE_DWORDX2 killed $vgpr0_vgpr1, killed %2, 0, 0, implicit $exec, implicit $flat_scr :: (store (s64))
+    S_ENDPGM 0
+
+...