[MachineInstr] Don't include debug uses in bundle header (PR52817)

[nikic]

Summary

Following https://github.com/llvm/llvm-project/issues/52817#issuecomment-1007635426, this excludes debug instructions when finalizing the bundle. Uses in debug instructions will no longer be included in the BUNDLE header.

This also ends up dropping the "internal" modifier for debug instructions -- not sure whether that's good or bad. If that should be retained, I can change the patch accordingly.

Fixes https://github.com/llvm/llvm-project/issues/52817.

[fhahn]

Adding a couple of reviewers that may be familiar with debug info + bundle interaction.

[MatzeB]

• Loosing the internal flag for uses that don't really read the register seems fine to me.
• Added a suggestion to try MachineOperand::readsReg() instead of checking for debug instructions.

llvm/lib/CodeGen/MachineInstrBundle.cpp

147–158

Does checking MO.readsReg() work as well? I looks like we may have a similar problem with undef operands.

[MatzeB]

Please try the suggestion. Either way LGTM.

[nikic]

llvm/lib/CodeGen/MachineInstrBundle.cpp

147–158

Nope, that doesn't work. It looks like MO.readsReg() returns true for DBG_VALUE uses.

llvm/lib/CodeGen/MachineInstrBundle.cpp

void llvm::finalizeBundle(MachineBasicBlock &MBB,
   SmallSet<Register, 8> DeadDefSet;
   SmallSet<Register, 16> KilledDefSet;
   SmallVector<Register, 8> ExternUses;
   SmallSet<Register, 8> ExternUseSet;
   SmallSet<Register, 8> KilledUseSet;
   SmallSet<Register, 8> UndefUseSet;
   SmallVector<MachineOperand*, 4> Defs;
   for (auto MII = FirstMI; MII != LastMI; ++MII) {
+    // Debug instructions have no effects to track.
+    if (MII->isDebugInstr())
+      continue;
+
     for (unsigned i = 0, e = MII->getNumOperands(); i != e; ++i) {
       MachineOperand &MO = MII->getOperand(i);
       if (!MO.isReg())
         continue;
       if (MO.isDef()) {
         Defs.push_back(&MO);
         continue;
       }
 
       Register Reg = MO.getReg();
       if (!Reg)
         continue;
 
       if (LocalDefSet.count(Reg)) {
         MO.setIsInternalRead();
         if (MO.isKill())

llvm/test/CodeGen/AMDGPU/postra-bundle-memops.mir

 body:             |
   bb.0:
     liveins: $vgpr3_vgpr4, $vgpr5_vgpr6
     ; GCN-LABEL: name: bundle_dbg_value_0
     ; GCN: liveins: $vgpr3_vgpr4, $vgpr5_vgpr6
     ; GCN-NEXT: {{  $}}
     ; GCN-NEXT: BUNDLE implicit-def $vgpr0, implicit-def $vgpr0_lo16, implicit-def $vgpr0_hi16, implicit-def $vgpr1, implicit-def $vgpr1_lo16, implicit-def $vgpr1_hi16, implicit $vgpr3_vgpr4, implicit $exec, implicit $vgpr5_vgpr6 {
     ; GCN-NEXT:   $vgpr0 = GLOBAL_LOAD_DWORD $vgpr3_vgpr4, 0, 0, implicit $exec
-    ; GCN-NEXT:   DBG_VALUE internal $vgpr0, 0, 0
+    ; GCN-NEXT:   DBG_VALUE $vgpr0, 0, 0
     ; GCN-NEXT:   $vgpr1 = GLOBAL_LOAD_DWORD $vgpr5_vgpr6, 0, 0, implicit $exec
     ; GCN-NEXT: }
     $vgpr0 = GLOBAL_LOAD_DWORD $vgpr3_vgpr4, 0, 0, implicit $exec
     DBG_VALUE $vgpr0, 0, 0
     $vgpr1 = GLOBAL_LOAD_DWORD $vgpr5_vgpr6, 0, 0, implicit $exec
 
 ...
 
 # Middle dbg_value should be bundled
 ---
 name: bundle_dbg_value_1
 tracksRegLiveness: true
 body:             |
   bb.0:
     liveins: $vgpr3_vgpr4, $vgpr5_vgpr6, $vgpr1
     ; GCN-LABEL: name: bundle_dbg_value_1
     ; GCN: liveins: $vgpr3_vgpr4, $vgpr5_vgpr6, $vgpr1
     ; GCN-NEXT: {{  $}}
-    ; GCN-NEXT: BUNDLE implicit-def $vgpr0, implicit-def $vgpr0_lo16, implicit-def $vgpr0_hi16, implicit-def $vgpr2, implicit-def $vgpr2_lo16, implicit-def $vgpr2_hi16, implicit $vgpr3_vgpr4, implicit $exec, implicit $vgpr1, implicit $vgpr5_vgpr6 {
+    ; GCN-NEXT: BUNDLE implicit-def $vgpr0, implicit-def $vgpr0_lo16, implicit-def $vgpr0_hi16, implicit-def $vgpr2, implicit-def $vgpr2_lo16, implicit-def $vgpr2_hi16, implicit $vgpr3_vgpr4, implicit $exec, implicit $vgpr5_vgpr6 {
     ; GCN-NEXT:   $vgpr0 = GLOBAL_LOAD_DWORD $vgpr3_vgpr4, 0, 0, implicit $exec
-    ; GCN-NEXT:   DBG_VALUE internal $vgpr0, 0, 0
+    ; GCN-NEXT:   DBG_VALUE $vgpr0, 0, 0
     ; GCN-NEXT:   DBG_VALUE $vgpr1, 0, 0
     ; GCN-NEXT:   $vgpr2 = GLOBAL_LOAD_DWORD $vgpr5_vgpr6, 0, 0, implicit $exec
     ; GCN-NEXT: }
     ; GCN-NEXT: DBG_VALUE $vgpr2, 0, 0
     $vgpr0 = GLOBAL_LOAD_DWORD $vgpr3_vgpr4, 0, 0, implicit $exec
     DBG_VALUE $vgpr0, 0, 0
     DBG_VALUE $vgpr1, 0, 0
     $vgpr2 = GLOBAL_LOAD_DWORD $vgpr5_vgpr6, 0, 0, implicit $exec
     DBG_VALUE $vgpr2, 0, 0
 ...
 
 # Starting and ending dbg_values should not be in the bundle
 ---
 name: bundle_dbg_value_2
 tracksRegLiveness: true
 body:             |
   bb.0:
     liveins: $vgpr3_vgpr4, $vgpr5_vgpr6, $vgpr1
     ; GCN-LABEL: name: bundle_dbg_value_2
     ; GCN: liveins: $vgpr3_vgpr4, $vgpr5_vgpr6, $vgpr1
     ; GCN-NEXT: {{  $}}
     ; GCN-NEXT: DBG_VALUE $vgpr1, 0, 0
     ; GCN-NEXT: BUNDLE implicit-def $vgpr0, implicit-def $vgpr0_lo16, implicit-def $vgpr0_hi16, implicit-def $vgpr2, implicit-def $vgpr2_lo16, implicit-def $vgpr2_hi16, implicit $vgpr3_vgpr4, implicit $exec, implicit $vgpr5_vgpr6 {
     ; GCN-NEXT:   $vgpr0 = GLOBAL_LOAD_DWORD $vgpr3_vgpr4, 0, 0, implicit $exec
-    ; GCN-NEXT:   DBG_VALUE internal $vgpr0, 0, 0
+    ; GCN-NEXT:   DBG_VALUE $vgpr0, 0, 0
     ; GCN-NEXT:   $vgpr2 = GLOBAL_LOAD_DWORD $vgpr5_vgpr6, 0, 0, implicit $exec
     ; GCN-NEXT: }
     ; GCN-NEXT: DBG_VALUE $vgpr2, 0, 0
     DBG_VALUE $vgpr1, 0, 0
     $vgpr0 = GLOBAL_LOAD_DWORD $vgpr3_vgpr4, 0, 0, implicit $exec
     DBG_VALUE $vgpr0, 0, 0
     $vgpr2 = GLOBAL_LOAD_DWORD $vgpr5_vgpr6, 0, 0, implicit $exec
     DBG_VALUE $vgpr2, 0, 0

llvm/test/CodeGen/Thumb2/mve-vpt-block-debug.mir

bb.0.entry:
     ; CHECK-NEXT: DBG_VALUE $q1, $noreg, !18, !DIExpression(), debug-location !21
     ; CHECK-NEXT: DBG_VALUE $q2, $noreg, !19, !DIExpression(), debug-location !21
     ; CHECK-NEXT: DBG_VALUE $q2, $noreg, !19, !DIExpression(), debug-location !21
     ; CHECK-NEXT: DBG_VALUE $noreg, $noreg, !20, !DIExpression(), debug-location !21
     ; CHECK-NEXT: BUNDLE implicit-def dead $vpr, implicit-def $q2, implicit-def $d4, implicit-def $s8, implicit-def $s9, implicit-def $d5, implicit-def $s10, implicit-def $s11, implicit killed $q1, implicit killed $q0, implicit killed $q2, debug-location !23 {
     ; CHECK-NEXT:   MVE_VPTv4s32 12, renamable $q1, renamable $q0, 10, implicit-def $vpr, debug-location !23
     ; CHECK-NEXT:   renamable $q2 = MVE_VADDi32 renamable $q0, renamable $q1, 1, internal renamable $vpr, $noreg, killed renamable $q2, debug-location !23
     ; CHECK-NEXT:   DBG_VALUE $noreg, $noreg, !20, !DIExpression(), debug-location !21
-    ; CHECK-NEXT:   DBG_VALUE internal $q2, $noreg, !19, !DIExpression(), debug-location !21
+    ; CHECK-NEXT:   DBG_VALUE $q2, $noreg, !19, !DIExpression(), debug-location !21
     ; CHECK-NEXT:   renamable $q2 = MVE_VADDi32 killed renamable $q0, killed renamable $q1, 2, internal killed renamable $vpr, $noreg, internal killed renamable $q2, debug-location !25
-    ; CHECK-NEXT:   DBG_VALUE internal $q2, $noreg, !19, !DIExpression(), debug-location !21
+    ; CHECK-NEXT:   DBG_VALUE $q2, $noreg, !19, !DIExpression(), debug-location !21
     ; CHECK-NEXT: }
     ; CHECK-NEXT: $q0 = MVE_VORR killed $q2, killed $q2, 0, $noreg, $noreg, undef $q0, debug-location !26
     ; CHECK-NEXT: tBX_RET 14 /* CC::al */, $noreg, implicit $q0, debug-location !26
     DBG_VALUE $q0, $noreg, !17, !DIExpression(), debug-location !21
     DBG_VALUE $q0, $noreg, !17, !DIExpression(), debug-location !21
     DBG_VALUE $q1, $noreg, !18, !DIExpression(), debug-location !21
     DBG_VALUE $q1, $noreg, !18, !DIExpression(), debug-location !21
     DBG_VALUE $q2, $noreg, !19, !DIExpression(), debug-location !21

llvm/test/CodeGen/Thumb2/pr52817.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=thumbv7-apple-ios9.0.0 -verify-machineinstrs < %s | FileCheck %s
+
+; Make sure machine verification does not fail due to incorrect bundle kill
+; flags.
+
+target triple = "thumbv7-apple-ios9.0.0"
+
+%struct.ham = type { [1024 x i32], %struct.zot }
+%struct.zot = type { [1 x i32], [1 x [32 x i32]] }
+
+define i32 @test(%struct.ham* %arg, %struct.zot* %arg1, i32* %arg2) #0 !dbg !6 {
+; CHECK-LABEL: test:
+; CHECK:       Lfunc_begin0:
+; CHECK-NEXT:    .file 1 "/" "test.cpp"
+; CHECK-NEXT:    .loc 1 50 0 @ test.cpp:50:0
+; CHECK-NEXT:  @ %bb.0: @ %bb
+; CHECK-NEXT:    push {r4, r5, r6, r7, lr}
+; CHECK-NEXT:    add r7, sp, #12
+; CHECK-NEXT:    str r8, [sp, #-4]!
+; CHECK-NEXT:    mov.w lr, #0
+; CHECK-NEXT:    mov.w r9, #1
+; CHECK-NEXT:    movw r12, #4100
+; CHECK-NEXT:    movs r3, #0
+; CHECK-NEXT:  LBB0_1: @ %bb3
+; CHECK-NEXT:    @ =>This Inner Loop Header: Depth=1
+; CHECK-NEXT:    adds r5, r3, #1
+; CHECK-NEXT:    str.w lr, [r2]
+; CHECK-NEXT:    cmp.w lr, #0
+; CHECK-NEXT:    add.w r4, r0, r5, lsl #2
+; CHECK-NEXT:    add.w r8, r4, r12
+; CHECK-NEXT:    lsl.w r4, r9, r3
+; CHECK-NEXT:    and.w r3, r3, r4
+; CHECK-NEXT:    add.w r4, r1, r5, lsl #2
+; CHECK-NEXT:    itte ne
+; CHECK-NEXT:    movne r6, #0
+; CHECK-NEXT:  Ltmp0:
+; CHECK-NEXT:    @DEBUG_VALUE: test:this <- [DW_OP_LLVM_arg 0, DW_OP_plus_uconst 135168, DW_OP_LLVM_arg 1, DW_OP_constu 4, DW_OP_mul, DW_OP_plus, DW_OP_plus_uconst 4, DW_OP_stack_value] $r0, $r5
+; CHECK-NEXT:    .loc 1 28 24 prologue_end @ test.cpp:28:24
+; CHECK-NEXT:    strne.w r6, [r8]
+; CHECK-NEXT:    moveq r6, #1
+; CHECK-NEXT:    ldr r4, [r4, #4]
+; CHECK-NEXT:    orrs r4, r6
+; CHECK-NEXT:    str.w r4, [r8]
+; CHECK-NEXT:    b LBB0_1
+; CHECK-NEXT:  Ltmp1:
+; CHECK-NEXT:  Lfunc_end0:
+bb:
+  br label %bb3
+
+bb3:                                              ; preds = %bb14, %bb
+  %tmp4 = phi i32 [ %tmp8, %bb14 ], [ 0, %bb ]
+  %tmp5 = add i32 %tmp4, 1
+  %tmp6 = shl nuw i32 1, %tmp4
+  %tmp8 = and i32 %tmp4, %tmp6
+  store i32 0, i32* %arg2, align 4
+  %tmp10 = getelementptr inbounds %struct.ham, %struct.ham* %arg, i32 0, i32 1, i32 1, i32 0, i32 %tmp5
+  br i1 undef, label %bb11, label %bb13
+
+bb11:                                             ; preds = %bb3
+  %tmp12 = load i32, i32* null, align 4
+  br label %bb14
+
+bb13:                                             ; preds = %bb3
+  call void @llvm.dbg.value(metadata !DIArgList(%struct.ham* %arg, i32 %tmp5), metadata !11, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_plus_uconst, 135168, DW_OP_LLVM_arg, 1, DW_OP_constu, 4, DW_OP_mul, DW_OP_plus, DW_OP_plus_uconst, 4, DW_OP_stack_value)), !dbg !14
+  store i32 0, i32* %tmp10, align 4, !dbg !16
+  br label %bb14
+
+bb14:                                             ; preds = %bb13, %bb11
+  %tmp15 = phi i32 [ 1, %bb11 ], [ 0, %bb13 ]
+  %tmp16 = getelementptr inbounds %struct.zot, %struct.zot* %arg1, i32 0, i32 1, i32 0, i32 %tmp5
+  %tmp17 = load i32, i32* %tmp16, align 4
+  %tmp18 = or i32 %tmp17, %tmp15
+  store i32 %tmp18, i32* %tmp10, align 4
+  br label %bb3
+}
+
+; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
+declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+
+attributes #0 = { "frame-pointer"="all" }
+attributes #1 = { nofree nosync nounwind readnone speculatable willreturn }
+
+!llvm.module.flags = !{!0, !1, !2}
+!llvm.dbg.cu = !{!3}
+
+!0 = !{i32 2, !"SDK Version", [2 x i32] [i32 15, i32 2]}
+!1 = !{i32 7, !"Dwarf Version", i32 4}
+!2 = !{i32 2, !"Debug Info Version", i32 3}
+!3 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !4, producer: "clang version 14.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !5, retainedTypes: !5, imports: !5, splitDebugInlining: false, nameTableKind: None, sysroot: "/")
+!4 = !DIFile(filename: "test.cpp", directory: "/")
+!5 = !{}
+!6 = distinct !DISubprogram(name: "test", linkageName: "test", scope: !7, file: !4, line: 49, type: !9, scopeLine: 50, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !3, declaration: !10, retainedNodes: !5)
+!7 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "BVGraph<__sanitizer::TwoLevelBitVector<1UL, __sanitizer::BasicBitVector<unsigned long> > >", scope: !8, file: !4, line: 25, size: 1083456, flags: DIFlagTypePassByValue, elements: !5, templateParams: !5, identifier: "_ZTSN11__sanitizer7BVGraphINS_17TwoLevelBitVectorILm1ENS_14BasicBitVectorImEEEEEE")
+!8 = !DINamespace(name: "__sanitizer", scope: null)
+!9 = !DISubroutineType(types: !5)
+!10 = !DISubprogram(name: "test", linkageName: "test", scope: !7, file: !4, line: 49, type: !9, scopeLine: 49, flags: DIFlagPublic | DIFlagPrototyped, spFlags: DISPFlagOptimized)
+!11 = !DILocalVariable(name: "this", arg: 1, scope: !6, type: !12, flags: DIFlagArtificial | DIFlagObjectPointer)
+!12 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !13, size: 32)
+!13 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "BasicBitVector<unsigned long>", scope: !8, file: !4, line: 22, size: 32, flags: DIFlagTypePassByValue, elements: !5, templateParams: !5, identifier: "_ZTSN11__sanitizer14BasicBitVectorImEE")
+!14 = !DILocation(line: 0, scope: !6, inlinedAt: !15)
+!15 = distinct !DILocation(line: 204, column: 23, scope: !6)
+!16 = !DILocation(line: 28, column: 24, scope: !6, inlinedAt: !15)