[llvm] [WebAssembly] Use DebugValueManager only when subprogram exists (PR #77978)
Heejin Ahn via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 12 14:57:59 PST 2024
https://github.com/aheejin updated https://github.com/llvm/llvm-project/pull/77978
>From a3bbe28d9fe67f2764edbe02bbcea511f81d1472 Mon Sep 17 00:00:00 2001
From: Heejin Ahn <aheejin at gmail.com>
Date: Fri, 12 Jan 2024 00:41:16 -0800
Subject: [PATCH 1/2] [WebAssembly] Use DebugValueManager only when subprogram
exists
We previously scanned the whole BB for `DBG_VALUE` instruction even when
the program doesn't have debug info, i.e., the function doesn't have a
subprogram associated with it, which can make compilation unnecessarily
slow. This disables `DebugValueManager` when a `DISubprogram` doesn't
exist for a function.
This only reduces unnecessary work in non-debug mode and does not change
output, so it's hard to add a test to test this behavior.
`cfg-stackify-dbg-skip.ll` test change is necessary because its
`DISubprogram` was not correctly linked with the function `foo`, so with
this PR the compiler incorrectly assumed function `foo` didn't have a
subprogram and the test started to fail.
Fixes https://github.com/emscripten-core/emscripten/issues/21048.
---
- Further thoughts:
This PR, which disables `DebugValueManager` in non-debug mode solves the
immediate problem at hand
(https://github.com/emscripten-core/emscripten/issues/21048), but this
means the example in that issue will still take a long time to compile
when `-g` is given.
The reason why
https://github.com/emscripten-core/emscripten/blob/main/test/printf/test_printf.c
(the example in
https://github.com/emscripten-core/emscripten/issues/21048) took really
long time was that file contained a single BB with more than 10,000
instructions, and `WebAssemblyDebugValueManager`'s constructor contains
a loop that's O(n^2):
https://github.com/llvm/llvm-project/blob/011ba725070360341f5473e88ebb4c956574805f/llvm/lib/Target/WebAssembly/WebAssemblyDebugValueManager.cpp#L32-L40
It was written this way to salvage as many `DBG_VALUE`s as possible.
For example, `MachineInstr::collectDebugValues` only collects
`DBG_VALUE`s that immediately follow a def:
https://github.com/llvm/llvm-project/blob/011ba725070360341f5473e88ebb4c956574805f/llvm/lib/CodeGen/MachineInstr.cpp#L2315-L2329
So that routine cannot detect something like
```ll
%def = ...
... some other instructions ...
DBG_VALUE %def ...
```
which occurs rather frequently. Our `WebAssemblyDebugValueManager`
constructor tried to capture those non-consecutive `DBG_VALUE`s as well,
but as a result, we scan the whole BB. This did not cause a significant
slowdown on programs with normal BB sizes, but can be a problem with
this kind of program that contains giant BBs with thousands of
instructions. Actually `DBG_VALUE`s that refer to `%def` can even be in
other BBs, which even us cannot capture. `DBG_INSTR_REF`
(https://llvm.org/docs/InstrRefDebugInfo.html) can be a better solution
for this but we don't use it (yet).
We can probably consider imposing some limits on the number of
instructions we can after a def. All this applies to the debug mode
compilation, so this is not directly related to what this PR does
though.
---
llvm/lib/Target/WebAssembly/WebAssemblyDebugValueManager.cpp | 4 ++++
llvm/test/CodeGen/WebAssembly/cfg-stackify-dbg-skip.ll | 4 ++--
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyDebugValueManager.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyDebugValueManager.cpp
index fd510f85a8a37a..a2a054127d5f65 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyDebugValueManager.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyDebugValueManager.cpp
@@ -17,11 +17,15 @@
#include "WebAssemblyMachineFunctionInfo.h"
#include "llvm/CodeGen/MachineInstr.h"
#include "llvm/IR/DebugInfoMetadata.h"
+#include "llvm/IR/Function.h"
using namespace llvm;
WebAssemblyDebugValueManager::WebAssemblyDebugValueManager(MachineInstr *Def)
: Def(Def) {
+ if (!Def->getMF()->getFunction().getSubprogram())
+ return;
+
// This code differs from MachineInstr::collectDebugValues in that it scans
// the whole BB, not just contiguous DBG_VALUEs, until another definition to
// the same register is encountered.
diff --git a/llvm/test/CodeGen/WebAssembly/cfg-stackify-dbg-skip.ll b/llvm/test/CodeGen/WebAssembly/cfg-stackify-dbg-skip.ll
index a10b9bfdc71af3..5a951d9043df29 100644
--- a/llvm/test/CodeGen/WebAssembly/cfg-stackify-dbg-skip.ll
+++ b/llvm/test/CodeGen/WebAssembly/cfg-stackify-dbg-skip.ll
@@ -15,7 +15,7 @@
target triple = "wasm32-unknown-unknown"
-define void @foo(i64 %arg) {
+define void @foo(i64 %arg) !dbg !37 {
start:
%val = trunc i64 %arg to i32
%cmp = icmp eq i32 %val, 0
@@ -39,7 +39,7 @@ declare void @llvm.dbg.value(metadata, metadata, metadata)
!22 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "&str", file: !6, size: 64, align: 32, elements: !{}, identifier: "111094d970b097647de579f9c509ef08")
!33 = !{i32 2, !"Debug Info Version", i32 3}
!35 = distinct !DILexicalBlock(scope: !37, file: !6, line: 357, column: 8)
-!37 = distinct !DISubprogram(name: "foobar", linkageName: "_fooba", scope: !38, file: !6, line: 353, type: !39, isLocal: true, isDefinition: true, scopeLine: 353, flags: DIFlagPrototyped, isOptimized: true, unit: !0, templateParams: !2, retainedNodes: !42)
+!37 = distinct !DISubprogram(name: "foo", scope: !6, file: !6, line: 353, type: !39, isLocal: true, isDefinition: true, scopeLine: 353, flags: DIFlagPrototyped, isOptimized: true, unit: !0, templateParams: !2, retainedNodes: !42)
!38 = !DINamespace(name: "ptr", scope: null)
!39 = !DISubroutineType(types: !2)
!42 = !{!46}
>From 0f942d6dc7bae6ddb9ec1e8a98de072ecbfcee14 Mon Sep 17 00:00:00 2001
From: Heejin Ahn <aheejin at gmail.com>
Date: Fri, 12 Jan 2024 14:57:36 -0800
Subject: [PATCH 2/2] More test changes
---
.../WebAssembly/dbg-value-move-clone.mir | 4 +--
.../WebAssembly/dbg-value-reg-stackify.mir | 35 ++++++++++++-------
2 files changed, 25 insertions(+), 14 deletions(-)
diff --git a/llvm/test/DebugInfo/WebAssembly/dbg-value-move-clone.mir b/llvm/test/DebugInfo/WebAssembly/dbg-value-move-clone.mir
index dd7b5a20f11160..506ed6a2b23c1d 100644
--- a/llvm/test/DebugInfo/WebAssembly/dbg-value-move-clone.mir
+++ b/llvm/test/DebugInfo/WebAssembly/dbg-value-move-clone.mir
@@ -20,7 +20,7 @@
declare void @foo(i32)
declare i32 @bar()
- define void @test(i64 %arg) {
+ define void @test(i64 %arg) !dbg !6 {
unreachable
}
@@ -32,7 +32,7 @@
!3 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "&str", file: !2, size: 64, align: 32, elements: !{}, identifier: "111094d970b097647de579f9c509ef08")
!4 = !{i32 2, !"Debug Info Version", i32 3}
!5 = distinct !DILexicalBlock(scope: !6, file: !2, line: 357, column: 8)
- !6 = distinct !DISubprogram(name: "testfoo", linkageName: "_testba", scope: !7, file: !2, line: 353, type: !8, isLocal: true, isDefinition: true, scopeLine: 353, flags: DIFlagPrototyped, isOptimized: true, unit: !0, templateParams: !1, retainedNodes: !9)
+ !6 = distinct !DISubprogram(name: "test", scope: !7, file: !2, line: 353, type: !8, isLocal: true, isDefinition: true, scopeLine: 353, flags: DIFlagPrototyped, isOptimized: true, unit: !0, templateParams: !1, retainedNodes: !9)
!7 = !DINamespace(name: "ptr", scope: null)
!8 = !DISubroutineType(types: !1)
!9 = !{!10}
diff --git a/llvm/test/DebugInfo/WebAssembly/dbg-value-reg-stackify.mir b/llvm/test/DebugInfo/WebAssembly/dbg-value-reg-stackify.mir
index bfb3f740c49307..230ba1228e1b08 100644
--- a/llvm/test/DebugInfo/WebAssembly/dbg-value-reg-stackify.mir
+++ b/llvm/test/DebugInfo/WebAssembly/dbg-value-reg-stackify.mir
@@ -8,44 +8,44 @@
declare void @use(i32)
declare void @use_2(i32, i32)
- define void @sink_simple() {
+ define void @sink_simple() !dbg !6 {
call void @llvm.dbg.value(metadata i32 0, metadata !5, metadata !DIExpression()), !dbg !10
call void @llvm.dbg.value(metadata i32 0, metadata !11, metadata !DIExpression()), !dbg !10
call void @llvm.dbg.value(metadata i32 0, metadata !12, metadata !DIExpression()), !dbg !10
call void @llvm.dbg.value(metadata i32 0, metadata !13, metadata !DIExpression()), !dbg !10
ret void
}
- define void @sink_non_consecutive() {
+ define void @sink_non_consecutive() !dbg !14 {
unreachable
}
- define void @dont_sink_above_def() {
+ define void @dont_sink_above_def() !dbg !15 {
unreachable
}
- define void @sink_to_same_place() {
+ define void @sink_to_same_place() !dbg !16 {
unreachable
}
- define void @cannot_sink_across_same_variable() {
+ define void @cannot_sink_across_same_variable() !dbg !17 {
unreachable
}
- define void @cannot_sink_across_same_variable2() {
+ define void @cannot_sink_across_same_variable2() !dbg !18 {
unreachable
}
- define void @can_sink_across_same_variable_with_same_const() {
+ define void @can_sink_across_same_variable_with_same_const() !dbg !19 {
unreachable
}
- define void @sink_multiple_defs() {
+ define void @sink_multiple_defs() !dbg !20 {
unreachable
}
- define void @clone_same_bb() {
+ define void @clone_same_bb() !dbg !21 {
unreachable
}
- define void @clone_different_bb() {
+ define void @clone_different_bb() !dbg !22 {
unreachable
}
- define void @tee_with_two_use_insts() {
+ define void @tee_with_two_use_insts() !dbg !23 {
unreachable
}
- define void @tee_with_one_inst_with_two_uses() {
+ define void @tee_with_one_inst_with_two_uses() !dbg !24 {
unreachable
}
declare void @llvm.dbg.value(metadata, metadata, metadata)
@@ -81,6 +81,17 @@
; CHECK: ![[VAR_C:[0-9]+]] = !DILocalVariable(name: "var_c"
!13 = !DILocalVariable(name: "var_d", scope: !6, file: !1, line: 2, type: !9)
; CHECK: ![[VAR_D:[0-9]+]] = !DILocalVariable(name: "var_d"
+ !14 = distinct !DISubprogram(name: "sink_non_consecutive", scope: !1, file: !1, line: 1, type: !7, scopeLine: 1, unit: !0)
+ !15 = distinct !DISubprogram(name: "dont_sink_above_def", scope: !1, file: !1, line: 1, type: !7, scopeLine: 1, unit: !0)
+ !16 = distinct !DISubprogram(name: "sink_to_same_place", scope: !1, file: !1, line: 1, type: !7, scopeLine: 1, unit: !0)
+ !17 = distinct !DISubprogram(name: "cannot_sink_across_same_variable", scope: !1, file: !1, line: 1, type: !7, scopeLine: 1, unit: !0)
+ !18 = distinct !DISubprogram(name: "cannot_sink_across_same_variable2", scope: !1, file: !1, line: 1, type: !7, scopeLine: 1, unit: !0)
+ !19 = distinct !DISubprogram(name: "can_sink_across_same_variable_with_same_const", scope: !1, file: !1, line: 1, type: !7, scopeLine: 1, unit: !0)
+ !20 = distinct !DISubprogram(name: "sink_multiple_defs", scope: !1, file: !1, line: 1, type: !7, scopeLine: 1, unit: !0)
+ !21 = distinct !DISubprogram(name: "clone_same_bb", scope: !1, file: !1, line: 1, type: !7, scopeLine: 1, unit: !0)
+ !22 = distinct !DISubprogram(name: "clone_different_bb", scope: !1, file: !1, line: 1, type: !7, scopeLine: 1, unit: !0)
+ !23 = distinct !DISubprogram(name: "tee_with_two_use_insts", scope: !1, file: !1, line: 1, type: !7, scopeLine: 1, unit: !0)
+ !24 = distinct !DISubprogram(name: "tee_with_one_inst_with_two_uses", scope: !1, file: !1, line: 1, type: !7, scopeLine: 1, unit: !0)
...
---
More information about the llvm-commits
mailing list