Skip to content

Commit 9792f2c

Browse files
kripkenkripken
authored
[DebugInfo] Add debug info to the values emitted in GlobalStructInference (#6709)
Previously the replacement select got the debug info, but we should also copy it to the values, as often optimizations lead to one of those values remaining by itself. Similar to #6652 in general form.
1 parent cdf8139 commit 9792f2c

File tree

2 files changed

+96
-15
lines changed

2 files changed

+96
-15
lines changed

‎src/passes/GlobalStructInference.cpp‎

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050

5151
#include <variant>
5252

53+
#include "ir/debuginfo.h"
5354
#include "ir/find_all.h"
5455
#include "ir/module-utils.h"
5556
#include "ir/names.h"
@@ -417,29 +418,36 @@ struct GlobalStructInference : public Pass {
417418
// Helper for optimization: Given a Value, returns what we should read
418419
// for it.
419420
auto getReadValue = [&](const Value& value) -> Expression* {
421+
Expression* ret;
420422
if (value.isConstant()) {
421423
// This is known to be a constant, so simply emit an expression for
422424
// that constant.
423-
return value.getConstant().makeExpression(wasm);
424-
}
425+
ret = value.getConstant().makeExpression(wasm);
426+
} else {
427+
// Otherwise, this is non-constant, so we are in the situation where
428+
// we want to un-nest the value out of the struct.new it is in. Note
429+
// that for later work, as we cannot add a global in parallel.
425430

426-
// Otherwise, this is non-constant, so we are in the situation where
427-
// we want to un-nest the value out of the struct.new it is in. Note
428-
// that for later work, as we cannot add a global in parallel.
431+
// There can only be one global in a value that is not constant,
432+
// which is the global we want to read from.
433+
assert(value.globals.size() == 1);
429434

430-
// There can only be one global in a value that is not constant, which
431-
// is the global we want to read from.
432-
assert(value.globals.size() == 1);
435+
// Create a global.get with temporary name, leaving only the
436+
// updating of the name to later work.
437+
auto* get = builder.makeGlobalGet(value.globals[0],
438+
value.getExpression()->type);
433439

434-
// Create a global.get with temporary name, leaving only the updating
435-
// of the name to later work.
436-
auto* get = builder.makeGlobalGet(value.globals[0],
437-
value.getExpression()->type);
440+
globalsToUnnest.emplace_back(
441+
GlobalToUnnest{value.globals[0], fieldIndex, get});
442+
443+
ret = get;
444+
}
438445

439-
globalsToUnnest.emplace_back(
440-
GlobalToUnnest{value.globals[0], fieldIndex, get});
446+
// This value replaces the struct.get, so it should have the same
447+
// source location.
448+
debuginfo::copyOriginalToReplacement(curr, ret, getFunction());
441449

442-
return get;
450+
return ret;
443451
};
444452

445453
// We have some globals (at least 2), and so must have at least one

‎test/lit/passes/gsi-debug.wast‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
2+
;; RUN: env BINARYEN_PRINT_FULL=1 foreach %s %t wasm-opt --gsi -all --closed-world -S -o - | filecheck %s
3+
4+
;; Test that debug info is copied to the values we replace a struct.get with.
5+
;; We use BINARYEN_PRINT_FULL=1 here because the select that contains the
6+
;; values also gets that debug info, and normally we omit debug info of children
7+
;; when it matches the parent (so we could not tell without
8+
;; BINARYEN_PRINT_FULL=1 whether the children had the debug info or not).
9+
;; (Another way to test this would be to run a followup optimization to remove
10+
;; the select, but that would be more complex.)
11+
12+
(module
13+
;; CHECK: (type $struct (struct (field i32)))
14+
(type $struct (struct i32))
15+
16+
;; CHECK: (type $1 (func (param (ref null $struct))))
17+
18+
;; CHECK: (global $global1 (ref $struct) (struct.new $struct
19+
;; CHECK-NEXT: (i32.const 42) (; i32 ;)
20+
;; CHECK-NEXT: ))
21+
(global $global1 (ref $struct) (struct.new $struct
22+
(i32.const 42)
23+
))
24+
25+
;; CHECK: (global $global2 (ref $struct) (struct.new $struct
26+
;; CHECK-NEXT: (i32.const 1337) (; i32 ;)
27+
;; CHECK-NEXT: ))
28+
(global $global2 (ref $struct) (struct.new $struct
29+
(i32.const 1337)
30+
))
31+
32+
;; A non-reference global does not confuse us.
33+
;; CHECK: (global $global-other i32 (i32.const 123456))
34+
(global $global-other i32 (i32.const 123456))
35+
36+
;; CHECK: (func $test (type $1) (param $struct (ref null $struct))
37+
;; CHECK-NEXT: ;;@ drop.c:10:1
38+
;; CHECK-NEXT: (drop
39+
;; CHECK-NEXT: ;;@ struct.c:20:2
40+
;; CHECK-NEXT: (select
41+
;; CHECK-NEXT: ;;@ struct.c:20:2
42+
;; CHECK-NEXT: (i32.const 42) (; i32 ;)
43+
;; CHECK-NEXT: ;;@ struct.c:20:2
44+
;; CHECK-NEXT: (i32.const 1337) (; i32 ;)
45+
;; CHECK-NEXT: ;;@
46+
;; CHECK-NEXT: (ref.eq
47+
;; CHECK-NEXT: ;;@
48+
;; CHECK-NEXT: (ref.as_non_null
49+
;; CHECK-NEXT: ;;@ local.c:30:3
50+
;; CHECK-NEXT: (local.get $struct) (; struct null ;)
51+
;; CHECK-NEXT: ) (; struct ;)
52+
;; CHECK-NEXT: ;;@
53+
;; CHECK-NEXT: (global.get $global1) (; struct ;)
54+
;; CHECK-NEXT: ) (; i32 ;)
55+
;; CHECK-NEXT: ) (; i32 ;)
56+
;; CHECK-NEXT: ) (; none ;)
57+
;; CHECK-NEXT: )
58+
(func $test (param $struct (ref null $struct))
59+
;; We can infer that this get can reference either $global1 or $global2,
60+
;; and nothing else (aside from a null), and can emit a select between
61+
;; those values. While doing so we copy the debug info as well to the
62+
;; values in the select.
63+
;;@ drop.c:10:1
64+
(drop
65+
;;@ struct.c:20:2
66+
(struct.get $struct 0
67+
;;@ local.c:30:3
68+
(local.get $struct)
69+
)
70+
)
71+
)
72+
)
73+

0 commit comments

Comments
 (0)