[llvm] [MachineCombiner] Don't count debug instructions in block size (PR #225765)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-aarch64

Author: Mikael Holmen (mikaelholmen)

<details>
<summary>Changes</summary>

The method MachineCombinerImpl::combineInstructions made decisions based on
 MBB->size() > inc_threshold
and since MachineBasicBlock::size() includes DBG_VALUE, the existence of debug info could affect the resulting code.

Use sizeWithoutDebugLargerThan() instead to get the same code with and without debug info.

I originally found this problem for my out-of-tree target. Then I used Claude Opus 5 to convert the mir test for my out-of-tree target to one for AArch64.

---
Full diff: https://github.com/llvm/llvm-project/pull/225765.diff


2 Files Affected:

- (modified) llvm/lib/CodeGen/MachineCombiner.cpp (+2-2) 
- (added) llvm/test/CodeGen/AArch64/machine-combiner-dbg-value.mir (+61) 


``````````diff
diff --git a/llvm/lib/CodeGen/MachineCombiner.cpp b/llvm/lib/CodeGen/MachineCombiner.cpp
index ec2a9efa7e09c..fd0a84e9fc514 100644
--- a/llvm/lib/CodeGen/MachineCombiner.cpp
+++ b/llvm/lib/CodeGen/MachineCombiner.cpp
@@ -632,7 +632,7 @@ bool MachineCombinerImpl::combineInstructions(MachineBasicBlock *MBB) {
       if (DoRegPressureReduce &&
           getCombinerObjective(P) ==
               CombinerObjective::MustReduceRegisterPressure) {
-        if (MBB->size() > inc_threshold) {
+        if (MBB->sizeWithoutDebugLargerThan(inc_threshold)) {
           // Use incremental depth updates for basic blocks above threshold
           IncrementalUpdate = true;
           LastUpdate = BlockIter;
@@ -678,7 +678,7 @@ bool MachineCombinerImpl::combineInstructions(MachineBasicBlock *MBB) {
                                     InstrIdxForVirtReg, P,
                                     !IncrementalUpdate) &&
             preservesResourceLen(MBB, BlockTrace, InsInstrs, DelInstrs)) {
-          if (MBB->size() > inc_threshold) {
+          if (MBB->sizeWithoutDebugLargerThan(inc_threshold)) {
             // Use incremental depth updates for basic blocks above treshold
             IncrementalUpdate = true;
             LastUpdate = BlockIter;
diff --git a/llvm/test/CodeGen/AArch64/machine-combiner-dbg-value.mir b/llvm/test/CodeGen/AArch64/machine-combiner-dbg-value.mir
new file mode 100644
index 0000000000000..2272dca3a599c
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/machine-combiner-dbg-value.mir
@@ -0,0 +1,61 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 6
+#
+# We should get the same result when machine-combiner sees debug info as when
+# it does not. "-mir-strip-debugify-only=0" is needed for mir-strip-debug to
+# remove all debug info.
+#
+# RUN: llc -mtriple=aarch64 -mcpu=neoverse-n2 -run-pass mir-strip-debug,machine-combiner -mir-strip-debugify-only=0 -machine-combiner-inc-threshold=7 %s -o - | FileCheck %s
+# RUN: llc -mtriple=aarch64 -mcpu=neoverse-n2 -run-pass machine-combiner,mir-strip-debug -mir-strip-debugify-only=0 -machine-combiner-inc-threshold=7 %s -o - | FileCheck %s
+
+--- |
+  define void @test(float %a, float %b) !dbg !5 {
+    ret void
+  }
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!2}
+  !2 = !{i32 2, !"Debug Info Version", i32 3}
+  !0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, emissionKind: FullDebug)
+  !1 = !DIFile(filename: "t.c", directory: "/")
+  !5 = distinct !DISubprogram(name: "test", scope: !1, file: !1, type: !6, spFlags: DISPFlagDefinition, unit: !0)
+  !6 = !DISubroutineType(types: !7)
+  !7 = !{null}
+  !9 = !DILocation(line: 0, scope: !5)
+  !10 = !DILocalVariable(name: "x", scope: !5)
+name:            test
+tracksRegLiveness: true
+body:             |
+  ; CHECK-LABEL: name: test
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT:   liveins: $s0, $s1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:fpr32 = COPY $s0
+  ; CHECK-NEXT:   [[COPY1:%[0-9]+]]:fpr32 = COPY $s1
+  ; CHECK-NEXT:   [[FADDSrr:%[0-9]+]]:fpr32 = nsz reassoc FADDSrr [[COPY]], [[COPY]], implicit $fpcr
+  ; CHECK-NEXT:   [[FADDSrr1:%[0-9]+]]:fpr32 = nsz reassoc FADDSrr [[COPY1]], [[COPY]], implicit $fpcr
+  ; CHECK-NEXT:   [[FADDSrr2:%[0-9]+]]:fpr32 = nsz reassoc FADDSrr [[FADDSrr]], killed [[FADDSrr1]], implicit $fpcr
+  ; CHECK-NEXT:   [[FADDSrr3:%[0-9]+]]:fpr32 = nsz reassoc FADDSrr [[FADDSrr2]], [[COPY1]], implicit $fpcr
+  ; CHECK-NEXT:   [[COPY2:%[0-9]+]]:fpr32 = COPY [[FADDSrr3]]
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[FADDSrr4:%[0-9]+]]:fpr32 = nsz reassoc FADDSrr [[COPY1]], [[COPY]], implicit $fpcr
+  ; CHECK-NEXT:   [[FADDSrr5:%[0-9]+]]:fpr32 = nsz reassoc FADDSrr [[COPY2]], [[FADDSrr4]], implicit $fpcr
+  ; CHECK-NEXT:   [[FADDSrr6:%[0-9]+]]:fpr32 = nsz reassoc FADDSrr [[FADDSrr5]], [[COPY2]], implicit $fpcr
+  ; CHECK-NEXT:   B %bb.1
+  bb.0:
+    liveins: $s0, $s1
+    DBG_VALUE 0, $noreg, !10, !DIExpression(), debug-location !9
+    %0:fpr32 = COPY $s0
+    %1:fpr32 = COPY $s1
+    %2:fpr32 = nsz reassoc FADDSrr %0, %0, implicit $fpcr
+    %3:fpr32 = nsz reassoc FADDSrr %2, %1, implicit $fpcr
+    %4:fpr32 = nsz reassoc FADDSrr %3, %0, implicit $fpcr
+    %5:fpr32 = nsz reassoc FADDSrr %4, %1, implicit $fpcr
+    %6:fpr32 = COPY %5
+  bb.1:
+    %7:fpr32 = nsz reassoc FADDSrr %1, %0, implicit $fpcr
+    %8:fpr32 = nsz reassoc FADDSrr %6, %7, implicit $fpcr
+    %9:fpr32 = nsz reassoc FADDSrr %8, %6, implicit $fpcr
+    B %bb.1

``````````

</details>


https://github.com/llvm/llvm-project/pull/225765