Fix instruction counting in post-RA scheduler

[rampitec]

Summary

That is possible to bundle instructions but have no leading
BUNDLE. In such a case post-RA scheduler miscalculates a number
of instructions and asserts. The bundle in the test was created
by the Greedy.

[arsenm]

Is this really a well formed bundle? i.e. is this really a verifier bug?

[rampitec]

In D85456#2200529, @arsenm wrote:

Is this really a well formed bundle? i.e. is this really a verifier bug?

That's a good question. Technically that is possible to create such IR. It might be OK post-RA?
For the context, it was created by the SplitEditor::buildSingleSubRegCopy() in the D30438.

[arsenm]

I think this is an invalid bundle.

In D85456#2200548, @rampitec wrote:

In D85456#2200529, @arsenm wrote:

Is this really a well formed bundle? i.e. is this really a verifier bug?

That's a good question. Technically that is possible to create such IR. It might be OK post-RA?
For the context, it was created by the SplitEditor::buildSingleSubRegCopy() in the D30438.

I'm inclined to think it's invalid. It looks like using bundles at all was a hack. I think VirtRegRewriter::expandCopyBundle just missed the case where a copy becomes a kill.

Can you add a second test where this weird kill bundle thing is produced?

[rampitec]

Added test resulting in KILL bundle.

[rampitec]

In D85456#2200952, @arsenm wrote:

I think this is an invalid bundle.

In D85456#2200548, @rampitec wrote:

In D85456#2200529, @arsenm wrote:

Is this really a well formed bundle? i.e. is this really a verifier bug?

That's a good question. Technically that is possible to create such IR. It might be OK post-RA?
For the context, it was created by the SplitEditor::buildSingleSubRegCopy() in the D30438.

I'm inclined to think it's invalid. It looks like using bundles at all was a hack. I think VirtRegRewriter::expandCopyBundle just missed the case where a copy becomes a kill.

Can you add a second test where this weird kill bundle thing is produced?

Added splitkit-copy-bundle.mir. It produces a bundle with KILLs.

What's interesting I have created a patch to properly bundle copies in the SplitKit, which has resulted in KILLs surviving until inst printer and crash.

[kerbowa]

Seems wrong to have a bundle without BUNDLE opcode but the change seems fine obviously. Since it's allowed we should probably just account for the possibility.

[arsenm]

In D85456#2201120, @kerbowa wrote:

Seems wrong to have a bundle without BUNDLE opcode but the change seems fine obviously. Since it's allowed we should probably just account for the possibility.

But is it? I think this is a failing of the verifier and it is not allowed

[rampitec]

In D85456#2201140, @arsenm wrote:

In D85456#2201120, @kerbowa wrote:

Seems wrong to have a bundle without BUNDLE opcode but the change seems fine obviously. Since it's allowed we should probably just account for the possibility.

But is it? I think this is a failing of the verifier and it is not allowed

It probably isn't. But there seems to be quite a few places which do not work with properly formed bundles. One of them is ExpandPostRA.

[rampitec]

In D85456#2201163, @rampitec wrote:

In D85456#2201140, @arsenm wrote:

In D85456#2201120, @kerbowa wrote:

Seems wrong to have a bundle without BUNDLE opcode but the change seems fine obviously. Since it's allowed we should probably just account for the possibility.

But is it? I think this is a failing of the verifier and it is not allowed

It probably isn't. But there seems to be quite a few places which do not work with properly formed bundles. One of them is ExpandPostRA.

As weird as it sounds VirtRegRewriter::expandCopyBundle() does not work with properly formed bundles as well...

[rampitec]

Here is the alternative: D85484. It properly forms bundles in the SplitKit.

[rampitec]

Abandon in favor of D85484.

llvm/lib/CodeGen/PostRASchedulerList.cpp

for (MachineBasicBlock::iterator I = Current; I != MBB.begin();) {
         Scheduler.schedule();
         Scheduler.exitRegion();
         Scheduler.EmitSchedule();
         Current = &MI;
         CurrentCount = Count;
         Scheduler.Observe(MI, CurrentCount);
       }
       I = MI;
-      if (MI.isBundle())
+      if (MI.isBundledWithSucc())
         Count -= MI.getBundleSize();
     }
     assert(Count == 0 && "Instruction count mismatch!");
     assert((MBB.begin() == Current || CurrentCount != 0) &&
            "Instruction count mismatch!");
     Scheduler.enterRegion(&MBB, MBB.begin(), Current, CurrentCount);
     Scheduler.setEndIndex(CurrentCount);
     Scheduler.schedule();

llvm/test/CodeGen/AMDGPU/bundled_insts_no_bundle.mir

+# RUN: llc -march=amdgcn -mcpu=gfx900 -run-pass=post-RA-sched -verify-machineinstrs -o - %s | FileCheck -check-prefix=GCN %s
+
+---
+# GCN-LABEL: name: bundled_insts_no_bundle
+# GCN:      entry:
+# GCN-NEXT:   $sgpr0 = KILL undef $sgpr0 {
+# GCN-NEXT:     $sgpr1 = KILL undef $sgpr1
+# GCN-NEXT:   }
+
+name: bundled_insts_no_bundle
+body:             |
+  bb.0.entry:
+
+    $sgpr0 = KILL undef $sgpr0 {
+      $sgpr1 = KILL undef $sgpr1
+    }
+
+...

llvm/test/CodeGen/AMDGPU/splitkit-copy-bundle.mir

+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -march=amdgcn -mcpu=gfx900 -run-pass=greedy,virtregrewriter,post-RA-sched -o - %s | FileCheck -check-prefix=GCN %s
+
+---
+name:            splitkit_copy_bundle
+tracksRegLiveness: true
+machineFunctionInfo:
+  stackPtrOffsetReg: '$sgpr32'
+body:             |
+  ; GCN-LABEL: name: splitkit_copy_bundle
+  ; GCN: bb.0:
+  ; GCN:   successors: %bb.1(0x80000000)
+  ; GCN:   renamable $sgpr69 = S_MOV_B32 -1
+  ; GCN:   renamable $sgpr68 = S_MOV_B32 -1
+  ; GCN:   renamable $sgpr36 = S_MOV_B32 0
+  ; GCN:   renamable $sgpr34_sgpr35 = IMPLICIT_DEF
+  ; GCN:   renamable $sgpr98_sgpr99 = IMPLICIT_DEF
+  ; GCN:   renamable $sgpr80_sgpr81_sgpr82_sgpr83_sgpr84_sgpr85_sgpr86_sgpr87_sgpr88_sgpr89_sgpr90_sgpr91_sgpr92_sgpr93_sgpr94_sgpr95 = KILL undef renamable $sgpr80_sgpr81_sgpr82_sgpr83_sgpr84_sgpr85_sgpr86_sgpr87_sgpr88_sgpr89_sgpr90_sgpr91_sgpr92_sgpr93_sgpr94_sgpr95 {
+  ; GCN:     renamable $sgpr96_sgpr97 = KILL undef renamable $sgpr96_sgpr97
+  ; GCN:   }
+  ; GCN: bb.1:
+  ; GCN:   successors: %bb.2(0x80000000)
+  ; GCN:   liveins: $sgpr36_sgpr37_sgpr38_sgpr39_sgpr40_sgpr41_sgpr42_sgpr43_sgpr44_sgpr45_sgpr46_sgpr47_sgpr48_sgpr49_sgpr50_sgpr51_sgpr52_sgpr53_sgpr54_sgpr55_sgpr56_sgpr57_sgpr58_sgpr59_sgpr60_sgpr61_sgpr62_sgpr63_sgpr64_sgpr65_sgpr66_sgpr67:0x0000000000000003, $sgpr68_sgpr69_sgpr70_sgpr71_sgpr72_sgpr73_sgpr74_sgpr75_sgpr76_sgpr77_sgpr78_sgpr79_sgpr80_sgpr81_sgpr82_sgpr83_sgpr84_sgpr85_sgpr86_sgpr87_sgpr88_sgpr89_sgpr90_sgpr91_sgpr92_sgpr93_sgpr94_sgpr95_sgpr96_sgpr97_sgpr98_sgpr99:0x0FFFFFFFFFFFFFFF, $sgpr34_sgpr35, $sgpr98_sgpr99
+  ; GCN:   renamable $sgpr70 = COPY renamable $sgpr68
+  ; GCN:   renamable $sgpr71 = COPY renamable $sgpr69
+  ; GCN:   renamable $sgpr72 = COPY renamable $sgpr68
+  ; GCN:   renamable $sgpr73 = COPY renamable $sgpr69
+  ; GCN:   renamable $sgpr74 = COPY renamable $sgpr68
+  ; GCN:   renamable $sgpr75 = COPY renamable $sgpr69
+  ; GCN:   renamable $sgpr76 = COPY renamable $sgpr68
+  ; GCN:   renamable $sgpr77 = COPY renamable $sgpr69
+  ; GCN:   renamable $sgpr78 = COPY renamable $sgpr68
+  ; GCN:   renamable $sgpr79 = COPY renamable $sgpr69
+  ; GCN:   renamable $sgpr80 = COPY renamable $sgpr68
+  ; GCN:   renamable $sgpr81 = COPY renamable $sgpr69
+  ; GCN:   renamable $sgpr82 = COPY renamable $sgpr68
+  ; GCN:   renamable $sgpr83 = COPY renamable $sgpr69
+  ; GCN:   renamable $sgpr84 = COPY renamable $sgpr68
+  ; GCN:   renamable $sgpr85 = COPY renamable $sgpr69
+  ; GCN:   renamable $sgpr86 = COPY renamable $sgpr68
+  ; GCN:   renamable $sgpr87 = COPY renamable $sgpr69
+  ; GCN:   renamable $sgpr88 = COPY renamable $sgpr68
+  ; GCN:   renamable $sgpr89 = COPY renamable $sgpr69
+  ; GCN:   renamable $sgpr90 = COPY renamable $sgpr68
+  ; GCN:   renamable $sgpr91 = COPY renamable $sgpr69
+  ; GCN:   renamable $sgpr92 = COPY renamable $sgpr68
+  ; GCN:   renamable $sgpr93 = COPY renamable $sgpr69
+  ; GCN:   renamable $sgpr94 = COPY renamable $sgpr68
+  ; GCN:   renamable $sgpr95 = COPY renamable $sgpr69
+  ; GCN:   renamable $sgpr96 = COPY renamable $sgpr68
+  ; GCN:   renamable $sgpr97 = COPY renamable $sgpr69
+  ; GCN:   renamable $sgpr37 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr38 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr39 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr40 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr41 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr42 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr43 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr44 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr45 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr46 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr47 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr48 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr49 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr50 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr51 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr52 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr53 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr54 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr55 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr56 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr57 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr58 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr59 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr60 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr61 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr62 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr63 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr64 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr65 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr66 = COPY renamable $sgpr36
+  ; GCN:   renamable $sgpr67 = COPY renamable $sgpr36
+  ; GCN: bb.2:
+  ; GCN:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; GCN:   liveins: $sgpr36_sgpr37_sgpr38_sgpr39_sgpr40_sgpr41_sgpr42_sgpr43_sgpr44_sgpr45_sgpr46_sgpr47_sgpr48_sgpr49_sgpr50_sgpr51_sgpr52_sgpr53_sgpr54_sgpr55_sgpr56_sgpr57_sgpr58_sgpr59_sgpr60_sgpr61_sgpr62_sgpr63_sgpr64_sgpr65_sgpr66_sgpr67:0x0000000000000003, $sgpr68_sgpr69_sgpr70_sgpr71_sgpr72_sgpr73_sgpr74_sgpr75_sgpr76_sgpr77_sgpr78_sgpr79_sgpr80_sgpr81_sgpr82_sgpr83_sgpr84_sgpr85_sgpr86_sgpr87_sgpr88_sgpr89_sgpr90_sgpr91_sgpr92_sgpr93_sgpr94_sgpr95_sgpr96_sgpr97_sgpr98_sgpr99:0x0FFFFFFFFFFFFFFF, $sgpr34_sgpr35, $sgpr98_sgpr99
+  ; GCN:   S_NOP 0, csr_amdgpu_highregs, implicit renamable $sgpr34_sgpr35, implicit renamable $sgpr98_sgpr99
+  ; GCN:   S_CBRANCH_VCCNZ %bb.1, implicit undef $vcc
+  ; GCN:   S_BRANCH %bb.2
+  bb.0:
+    %0:sreg_64 = IMPLICIT_DEF
+    %1:sreg_64 = IMPLICIT_DEF
+    undef %2.sub1:sgpr_1024 = S_MOV_B32 -1
+    %2.sub0:sgpr_1024 = S_MOV_B32 -1
+    undef %3.sub0:sgpr_1024 = S_MOV_B32 0
+
+  bb.1:
+    %2.sub2:sgpr_1024 = COPY %2.sub0
+    %2.sub3:sgpr_1024 = COPY %2.sub1
+    %2.sub4:sgpr_1024 = COPY %2.sub0
+    %2.sub5:sgpr_1024 = COPY %2.sub1
+    %2.sub6:sgpr_1024 = COPY %2.sub0
+    %2.sub7:sgpr_1024 = COPY %2.sub1
+    %2.sub8:sgpr_1024 = COPY %2.sub0
+    %2.sub9:sgpr_1024 = COPY %2.sub1
+    %2.sub10:sgpr_1024 = COPY %2.sub0
+    %2.sub11:sgpr_1024 = COPY %2.sub1
+    %2.sub12:sgpr_1024 = COPY %2.sub0
+    %2.sub13:sgpr_1024 = COPY %2.sub1
+    %2.sub14:sgpr_1024 = COPY %2.sub0
+    %2.sub15:sgpr_1024 = COPY %2.sub1
+    %2.sub16:sgpr_1024 = COPY %2.sub0
+    %2.sub17:sgpr_1024 = COPY %2.sub1
+    %2.sub18:sgpr_1024 = COPY %2.sub0
+    %2.sub19:sgpr_1024 = COPY %2.sub1
+    %2.sub20:sgpr_1024 = COPY %2.sub0
+    %2.sub21:sgpr_1024 = COPY %2.sub1
+    %2.sub22:sgpr_1024 = COPY %2.sub0
+    %2.sub23:sgpr_1024 = COPY %2.sub1
+    %2.sub24:sgpr_1024 = COPY %2.sub0
+    %2.sub25:sgpr_1024 = COPY %2.sub1
+    %2.sub26:sgpr_1024 = COPY %2.sub0
+    %2.sub27:sgpr_1024 = COPY %2.sub1
+    %2.sub28:sgpr_1024 = COPY %2.sub0
+    %2.sub29:sgpr_1024 = COPY %2.sub1
+    %3.sub1:sgpr_1024 = COPY %3.sub0
+    %3.sub2:sgpr_1024 = COPY %3.sub0
+    %3.sub3:sgpr_1024 = COPY %3.sub0
+    %3.sub4:sgpr_1024 = COPY %3.sub0
+    %3.sub5:sgpr_1024 = COPY %3.sub0
+    %3.sub6:sgpr_1024 = COPY %3.sub0
+    %3.sub7:sgpr_1024 = COPY %3.sub0
+    %3.sub8:sgpr_1024 = COPY %3.sub0
+    %3.sub9:sgpr_1024 = COPY %3.sub0
+    %3.sub10:sgpr_1024 = COPY %3.sub0
+    %3.sub11:sgpr_1024 = COPY %3.sub0
+    %3.sub12:sgpr_1024 = COPY %3.sub0
+    %3.sub13:sgpr_1024 = COPY %3.sub0
+    %3.sub14:sgpr_1024 = COPY %3.sub0
+    %3.sub15:sgpr_1024 = COPY %3.sub0
+    %3.sub16:sgpr_1024 = COPY %3.sub0
+    %3.sub17:sgpr_1024 = COPY %3.sub0
+    %3.sub18:sgpr_1024 = COPY %3.sub0
+    %3.sub19:sgpr_1024 = COPY %3.sub0
+    %3.sub20:sgpr_1024 = COPY %3.sub0
+    %3.sub21:sgpr_1024 = COPY %3.sub0
+    %3.sub22:sgpr_1024 = COPY %3.sub0
+    %3.sub23:sgpr_1024 = COPY %3.sub0
+    %3.sub24:sgpr_1024 = COPY %3.sub0
+    %3.sub25:sgpr_1024 = COPY %3.sub0
+    %3.sub26:sgpr_1024 = COPY %3.sub0
+    %3.sub27:sgpr_1024 = COPY %3.sub0
+    %3.sub28:sgpr_1024 = COPY %3.sub0
+    %3.sub29:sgpr_1024 = COPY %3.sub0
+    %3.sub30:sgpr_1024 = COPY %3.sub0
+    %3.sub31:sgpr_1024 = COPY %3.sub0
+
+  bb.2:
+    S_NOP 0, implicit %0, implicit %1, csr_amdgpu_highregs
+    S_CBRANCH_VCCNZ %bb.1, implicit undef $vcc
+    S_BRANCH %bb.2
+
+...