[llvm] [XRay] Don't count debug instructions towards the instruction threshold (PR #219100)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-x86

Author: Alexey Milovidov (alexey-milovidov)

<details>
<summary>Changes</summary>

`XRayInstrumentation` decides whether a loop-free function is too small to instrument by counting MachineInstrs:

```cpp
    // Count the number of MachineInstr`s in MachineFunction
    uint64_t MICount = 0;
    for (const auto &MBB : MF)
      MICount += MBB.size();

    bool TooFewInstrs = MICount < XRayThreshold;
```

`MachineBasicBlock::size()` counts the debug pseudo-instructions too, so building with debug info instruments functions that the very same source built without it leaves alone. Two consequences:

* the binary that ships is not the binary that was tested, and its code layout differs;
* a size or layout comparison between a `-g` build and a `-g0` build of one commit shows a difference that no source change caused.

Measured on ClickHouse (aarch64, `-O3 -flto=thin -fxray-instrument -fxray-modes=none -fxray-instrumentation-bundle=function`, default threshold of 200), comparing a `-g` build against a `-g0` build, both after `strip --strip-debug`:

| section | Δ bytes |
|---|---:|
| `xray_instr_map` | +925,024 |
| `xray_fn_idx` | +176,448 |
| `.text` (entry/exit sled NOPs plus function alignment) | +989,952 |
| `.symtab` | +1,059,096 |
| `.strtab` | +39,895 |
| `.eh_frame` / `.gcc_except_table` / `.eh_frame_hdr` | +16,436 |

That is 3.06 MiB on a 712 MiB binary (0.43%), and it all reconciles to one number: `xray_fn_idx` grows by exactly 11,028 entries of 16 bytes, i.e. **11,028 functions instrumented only because the module carries debug info**. The `.symtab` growth is 2 × 11,028 `.Lxray_sleds_start*`/`.Lxray_fn_idx*` labels plus 22,063 AArch64 `$d` mapping symbols for the new per-function sections, and `.text` is 28,907 sleds of 32 bytes.

This patch skips debug instructions in the count. `-g0` behaviour is unchanged by construction, so no existing test moves.

The added test pins the boundary: a function with two real instructions and three debug records against a threshold of four. It is instrumented before this patch and not after, and a companion function above the threshold is still instrumented either way.

I kept the predicate at `MI.isDebugInstr()` rather than `MI.isMetaInstruction()` deliberately: the latter would also stop counting `IMPLICIT_DEF`, `KILL`, `CFI_INSTRUCTION` and friends, which would change which functions get instrumented in builds without debug info as well. Happy to switch if reviewers prefer that.


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


2 Files Affected:

- (modified) llvm/lib/CodeGen/XRayInstrumentation.cpp (+6-2) 
- (added) llvm/test/CodeGen/X86/xray-instruction-threshold-debug-info.ll (+45) 


``````````diff
diff --git a/llvm/lib/CodeGen/XRayInstrumentation.cpp b/llvm/lib/CodeGen/XRayInstrumentation.cpp
index f23357ef0bd17..0a69e7d05fe5e 100644
--- a/llvm/lib/CodeGen/XRayInstrumentation.cpp
+++ b/llvm/lib/CodeGen/XRayInstrumentation.cpp
@@ -217,10 +217,14 @@ bool XRayInstrumentation::run(MachineFunction &MF) {
     if (XRayThreshold == std::numeric_limits<uint64_t>::max())
       return false;
 
-    // Count the number of MachineInstr`s in MachineFunction
+    // Count the number of MachineInstr`s in MachineFunction, skipping debug
+    // instructions: they are not code, so whether a function is instrumented
+    // must not depend on whether the module was built with debug info.
     uint64_t MICount = 0;
     for (const auto &MBB : MF)
-      MICount += MBB.size();
+      MICount += llvm::count_if(MBB.instrs(), [](const MachineInstr &MI) {
+        return !MI.isDebugInstr();
+      });
 
     bool TooFewInstrs = MICount < XRayThreshold;
 
diff --git a/llvm/test/CodeGen/X86/xray-instruction-threshold-debug-info.ll b/llvm/test/CodeGen/X86/xray-instruction-threshold-debug-info.ll
new file mode 100644
index 0000000000000..5d5d8647ba63b
--- /dev/null
+++ b/llvm/test/CodeGen/X86/xray-instruction-threshold-debug-info.ll
@@ -0,0 +1,45 @@
+; Debug instructions are not code, so they must not count towards the XRay
+; instruction threshold: whether a function gets instrumented has to be the
+; same whether or not the module is built with debug info.
+;
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s
+
+; Two real instructions and three debug records, with a threshold of four: the
+; function stays uninstrumented.
+define i32 @below_threshold(i32 %a) nounwind uwtable "xray-instruction-threshold"="4" !dbg !4 {
+entry:
+    #dbg_value(i32 %a, !7, !DIExpression(), !10)
+    #dbg_value(i32 %a, !8, !DIExpression(), !10)
+    #dbg_value(i32 %a, !9, !DIExpression(), !10)
+  ret i32 %a, !dbg !10
+}
+
+; CHECK-LABEL: below_threshold:
+; CHECK-NOT:   xray_sled
+
+; The same function over the threshold is still instrumented.
+define i32 @above_threshold(i32 %a) nounwind uwtable "xray-instruction-threshold"="2" !dbg !11 {
+entry:
+  ret i32 %a, !dbg !12
+}
+
+; CHECK-LABEL: above_threshold:
+; CHECK:       xray_sled_0:
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
+!1 = !DIFile(filename: "t.c", directory: "/")
+!2 = !{i32 2, !"Debug Info Version", i32 3}
+!3 = !{i32 7, !"Dwarf Version", i32 5}
+!4 = distinct !DISubprogram(name: "below_threshold", scope: !1, file: !1, line: 1, type: !5, scopeLine: 1, spFlags: DISPFlagDefinition, unit: !0)
+!5 = !DISubroutineType(types: !6)
+!6 = !{!13, !13}
+!7 = !DILocalVariable(name: "x", scope: !4, file: !1, line: 1, type: !13)
+!8 = !DILocalVariable(name: "y", scope: !4, file: !1, line: 1, type: !13)
+!9 = !DILocalVariable(name: "z", scope: !4, file: !1, line: 1, type: !13)
+!10 = !DILocation(line: 1, column: 1, scope: !4)
+!11 = distinct !DISubprogram(name: "above_threshold", scope: !1, file: !1, line: 2, type: !5, scopeLine: 2, spFlags: DISPFlagDefinition, unit: !0)
+!12 = !DILocation(line: 2, column: 1, scope: !11)
+!13 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)

``````````

</details>


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