[RegisterCoalescer] Avoid "Use not jointly dominated by defs" in removePartialRedundancy

[tpr]

Summary

removePartialRedundancy was using extendToIndices on each subrange
without passing in an Undefs vector containing main reg defs that are
undef in the subrange, causing the above assert.

Unfortunately I can only reproduce this in an ll test. Turning it into a
mir test makes the problem go away.

Change-Id: I4f3ba2afb20d79f9467afb3d882f0328d38531be

[qcolombet]

Unfortunately I can only reproduce this in an ll test. Turning it into a mir test makes the problem go away.

Why is that?

[uabelho]

test/CodeGen/AMDGPU/regcoal-removepartial-redundancy-not-jointly-dominated.ll

3 ↗

(On Diff #164622)

I tried running the test case on trunk now and it doesn't trigger the verifier even without the fix in this patch so something must have changed so the "wanted" situation doesn't happen with this ll-input anymore.

[tpr]

Hmm, me too for trunk. It only fails on our fork that is 340950 plus local changes, so the code generated by instruction selection is probably different.

I will try again to turn it into a mir test, and/or determine why that doesn't work.

[tpr]

V2: mir test

[tpr]

1. That test only fails without this fix if you have the fix in D51257.

2. I finally tracked down why I couldn't get a mir test before: Compiling a ll file as usual, arriving at RegisterCoalescer, the numbering of the MBBs was not quite in order. Using -stop-before names the MBBs in the mir according to those out-of-order numbers, but reading the mir in again with -run-pass fails to preserve it. Instead it renumbers the MBBs in order. RegisterCoalescing processes MBBs with the same loop depth and critical edgeness and connectedness in numbering order, so the pass behaved differently.

[qcolombet]

I finally tracked down why I couldn't get a mir test before: Compiling a ll file as usual, arriving at RegisterCoalescer, the numbering of the MBBs was not quite in order. Using -stop-before names the MBBs in the mir according to those out-of-order numbers, but reading the mir in again with -run-pass fails to preserve it. Instead it renumbers the MBBs in order. RegisterCoalescing processes MBBs with the same loop depth and critical edgeness and connectedness in numbering order, so the pass behaved differently.

Could you file a public report for this issue?

test/CodeGen/AMDGPU/regcoal-removepartial-redundancy-not-jointly-dominated.mir

8

Please check that the code after coalescing is correct.
Eg., the pass could silently crash and produce an empty function, the test would still pass!

[foad]

If I apply this now, the new test fails with:

*** Bad machine code: Too few operands ***
- function:    _amdgpu_ps_main
- basic block: %bb.6  (0x53b7738)
- instruction: %8:sreg_32_xm0_xexec = S_BUFFER_LOAD_DWORD_IMM undef %9:sreg_128, 2708, 0 :: (dereferenceable invariant load 4)
5 operands expected, but 4 given.

Does this mean that the format of buffer load instructions has changed since you wrote this mir test?

[foad]

Answering my own question: yes S_BUFFER_LOAD_DWORD_IMM needs an extra 0 operand because of a6322941ffc15275e837435eee7aabba0ca15211 and S_ENDPGM also needs a 0 operand as in 7df225dfc25adc8371188dc57f3adf300b0bd697.

lib/CodeGen/RegisterCoalescer.cpp

for (LiveInterval::SubRange &SR : IntB.subranges()) {
     for (unsigned I = 0; I != EndPoints.size(); ) {
       if (SlotIndex::isSameInstr(EndPoints[I], CopyIdx)) {
         EndPoints[I] = EndPoints.back();
         EndPoints.pop_back();
         continue;
       }
       ++I;
     }
-    LIS->extendToIndices(SR, EndPoints);
+    SmallVector<SlotIndex, 8> Undefs;
+    IntB.computeSubRangeUndefs(Undefs, SR.LaneMask, *MRI, *LIS->getSlotIndexes());
+    LIS->extendToIndices(SR, EndPoints, Undefs);
   }
   // If any dead defs were extended, truncate them.
   shrinkToUses(&IntB);
 
   // Finally, update the live-range of IntA.
   shrinkToUses(&IntA);
   return true;
 }

test/CodeGen/AMDGPU/regcoal-removepartial-redundancy-not-jointly-dominated.mir

+# RUN: llc -march=amdgcn -mcpu=gfx803 -run-pass simple-register-coalescing -o - %s | FileCheck -check-prefix=GCN %s
+#
+# This test provokes
+#   Use of $noreg does not have a corresponding definition on every path:
+#   LLVM ERROR: Use not jointly dominated by defs.
+# without the associated fix.
+#
+# GCN: body:
+
+---
+name:            _amdgpu_ps_main
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    S_CBRANCH_SCC1 %bb.32, implicit undef $scc
+    S_BRANCH %bb.1
+  
+  bb.1:
+    successors: %bb.2, %bb.32
+  
+    S_CBRANCH_SCC1 %bb.32, implicit undef $scc
+    S_BRANCH %bb.2
+  
+  bb.2:
+    successors: %bb.3, %bb.4
+  
+    %20:sreg_32_xm0 = S_MOV_B32 0
+    undef %21.sub0:sreg_256 = COPY %20
+    %21.sub1:sreg_256 = COPY %20
+    %21.sub2:sreg_256 = COPY %20
+    %21.sub3:sreg_256 = COPY %20
+    %21.sub4:sreg_256 = COPY %20
+    %21.sub5:sreg_256 = COPY %20
+    %21.sub6:sreg_256 = COPY %20
+    %21.sub7:sreg_256 = COPY killed %20
+    %0:vreg_128 = IMAGE_LOAD_V4_V2 undef %22:vreg_64, killed %21, 15, -1, 0, 0, 0, 0, 0, 0, 0, implicit $exec :: (dereferenceable load 16 from constant-pool, addrspace 4)
+    S_CBRANCH_SCC1 %bb.4, implicit undef $scc
+    S_BRANCH %bb.3
+  
+  bb.3:
+  
+  bb.4:
+    successors: %bb.35, %bb.5
+  
+    %34:vgpr_32 = V_SUB_F32_e32 target-flags(amdgpu-gotprel32-hi) 1065353216, killed %0.sub2, implicit $exec
+    %36:vgpr_32 = V_MUL_F32_e32 0, killed %34, implicit $exec
+    dead %73:vgpr_32 = V_MOV_B32_e32 2143289344, implicit $exec
+    undef %72.sub0:vreg_128 = COPY %36
+    %72.sub1:vreg_128 = COPY %36
+    %72.sub2:vreg_128 = COPY killed %36
+    S_CBRANCH_SCC0 %bb.5, implicit undef $scc
+    S_BRANCH %bb.35
+  
+  bb.5:
+    S_CBRANCH_SCC0 %bb.6, implicit undef $scc
+    S_BRANCH %bb.34
+  
+  bb.6:
+    %40:sreg_32_xm0_xexec = S_BUFFER_LOAD_DWORD_IMM undef %41:sreg_128, 2708, 0 :: (dereferenceable invariant load 4)
+    %43:sreg_64_xexec = V_CMP_NE_U32_e64 0, killed %40, implicit $exec
+    %87:vreg_128 = COPY killed %72
+  
+  bb.7:
+    successors: %bb.7(0x7c000000), %bb.8(0x04000000)
+  
+    %74:vreg_128 = COPY killed %87
+    %48:vgpr_32 = nnan arcp contract reassoc V_MAC_F32_e64 0, target-flags(amdgpu-gotprel) 0, 0, undef %44.sub0:sreg_128, 0, undef %48, 0, 0, implicit $exec
+    %7:vgpr_32 = V_RCP_F32_e32 killed %48, implicit $exec
+    dead %49:vgpr_32 = V_MUL_F32_e32 0, killed %7, implicit $exec
+    %76:vreg_128 = COPY killed %74
+    %76.sub3:vreg_128 = COPY undef %49
+    %50:sreg_64 = S_AND_B64 $exec, %43, implicit-def dead $scc
+    $vcc = COPY killed %50
+    %75:vreg_128 = COPY %76
+    %87:vreg_128 = COPY killed %75
+    S_CBRANCH_VCCNZ %bb.7, implicit killed $vcc
+    S_BRANCH %bb.8
+  
+  bb.8:
+    %88:vreg_128 = COPY killed %76
+  
+  bb.9:
+    %77:vreg_128 = COPY killed %88
+    %89:vreg_128 = COPY killed %77
+  
+  bb.10:
+    %78:vreg_128 = COPY killed %89
+    S_CBRANCH_SCC1 %bb.18, implicit undef $scc
+    S_BRANCH %bb.11
+  
+  bb.11:
+    successors: %bb.12, %bb.18
+  
+    S_CBRANCH_SCC1 %bb.18, implicit undef $scc
+    S_BRANCH %bb.12
+  
+  bb.12:
+    successors: %bb.13, %bb.17
+  
+    S_CBRANCH_SCC1 %bb.17, implicit undef $scc
+    S_BRANCH %bb.13
+  
+  bb.13:
+    S_CBRANCH_SCC1 %bb.16, implicit undef $scc
+    S_BRANCH %bb.14
+  
+  bb.14:
+    S_CBRANCH_SCC1 %bb.16, implicit undef $scc
+    S_BRANCH %bb.15
+  
+  bb.15:
+  
+  bb.16:
+  
+  bb.17:
+  
+  bb.18:
+    successors: %bb.36, %bb.19
+  
+    S_CBRANCH_SCC0 %bb.19, implicit undef $scc
+    S_BRANCH %bb.36
+  
+  bb.19:
+    S_CBRANCH_SCC0 %bb.20, implicit undef $scc
+    S_BRANCH %bb.37
+  
+  bb.20:
+    S_CBRANCH_SCC1 %bb.24, implicit undef $scc
+    S_BRANCH %bb.21
+  
+  bb.21:
+    successors: %bb.22, %bb.23
+  
+    S_CBRANCH_SCC1 %bb.23, implicit undef $scc
+    S_BRANCH %bb.22
+  
+  bb.22:
+  
+  bb.23:
+  
+  bb.24:
+    dead %52:sreg_32_xm0 = S_MOV_B32 0
+    %79:vreg_128 = COPY killed %78
+    %79.sub3:vreg_128 = COPY undef %52
+    %90:vreg_128 = COPY killed %79
+  
+  bb.25:
+    %80:vreg_128 = COPY killed %90
+    S_CBRANCH_SCC1 %bb.32, implicit undef $scc
+    S_BRANCH %bb.26
+  
+  bb.26:
+    S_CBRANCH_SCC1 %bb.32, implicit undef $scc
+    S_BRANCH %bb.27
+  
+  bb.27:
+    %54:vgpr_32 = V_MOV_B32_e32 1050868099, implicit $exec
+    dead %55:vgpr_32 = V_MUL_F32_e32 %80.sub2, %54, implicit $exec
+    dead %57:vgpr_32 = V_MUL_F32_e32 %80.sub1, %54, implicit $exec
+    dead %59:vgpr_32 = V_MUL_F32_e32 killed %80.sub0, killed %54, implicit $exec
+    dead %82:vgpr_32 = V_MOV_B32_e32 2143289344, implicit $exec
+    dead %81:vreg_128 = IMPLICIT_DEF
+    dead %62:sreg_32_xm0 = S_MOV_B32 0
+    %64:sreg_64 = S_AND_B64 $exec, 0, implicit-def dead $scc
+    %91:vreg_128 = IMPLICIT_DEF
+  
+  bb.28:
+    dead %83:vreg_128 = COPY killed %91
+    %92:vreg_128 = IMPLICIT_DEF
+    S_CBRANCH_SCC1 %bb.31, implicit undef $scc
+    S_BRANCH %bb.29
+  
+  bb.29:
+    successors: %bb.30(0x7c000000), %bb.33(0x04000000)
+  
+    $vcc = COPY %64
+    S_CBRANCH_VCCNZ %bb.33, implicit killed $vcc
+  
+  bb.30:
+    dead undef %85.sub3:vreg_128 = COPY undef %62
+    %92:vreg_128 = IMPLICIT_DEF
+  
+  bb.31:
+    dead %86:vreg_128 = COPY killed %92
+    dead %84:vreg_128 = COPY undef %86
+    %91:vreg_128 = IMPLICIT_DEF
+    S_BRANCH %bb.28
+  
+  bb.32:
+    S_ENDPGM
+  
+  bb.33:
+    S_ENDPGM
+
+  bb.34:
+    %88:vreg_128 = COPY killed %72
+    S_BRANCH %bb.9
+  
+  bb.35:
+    %89:vreg_128 = COPY killed %72
+    S_BRANCH %bb.10
+  
+  bb.36:
+    %90:vreg_128 = COPY killed %78
+    S_BRANCH %bb.25
+  
+  bb.37:
+    %90:vreg_128 = COPY killed %78
+    S_BRANCH %bb.25
+  
+...