Skip to content

Commit 73bbbab

Browse files
authored
Simplify and consolidate type printing (#5816)
When printing Binaryen IR, we previously generated names for unnamed heap types based on their structure. This was useful for seeing the structure of simple types at a glance without having to separately go look up their definitions, but it also had two problems: 1. The same name could be generated for multiple types. The generated names did not take into account rec group structure or finality, so types that differed only in these properties would have the same name. Also, generated type names were limited in length, so very large types that shared only some structure could also end up with the same names. Using the same name for multiple types produces incorrect and unparsable output. 2. The generated names were not useful beyond the most trivial examples. Even with length limits, names for nontrivial types were extremely long and visually noisy, which made reading disassembled real-world code more challenging. Fix these problems by emitting simple indexed names for unnamed heap types instead. This regresses readability for very simple examples, but the trade off is worth it. This change also reduces the number of type printing systems we have by one. Previously we had the system in Print.cpp, but we had another, more general and extensible system in wasm-type-printing.h and wasm-type.cpp as well. Remove the old type printing system from Print.cpp and replace it with a much smaller use of the new system. This requires significant refactoring of Print.cpp so that PrintExpressionContents object now holds a reference to a parent PrintSExpression object that holds the type name state. This diff is very large because almost every test output changed slightly. To minimize the diff and ease review, change the type printer in wasm-type.cpp to behave the same as the old type printer in Print.cpp except for the differences in name generation. These changes will be reverted in much smaller PRs in the future to generally improve how types are printed.
1 parent da8937a commit 73bbbab

File tree

530 files changed

+17567
-17794
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

530 files changed

+17567
-17794
lines changed

src/passes/Print.cpp

Lines changed: 978 additions & 1210 deletions
Large diffs are not rendered by default.

src/wasm/wasm-type.cpp

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,8 @@ struct TypePrinter {
155155
std::ostream& print(const Tuple& tuple);
156156
std::ostream& print(const Field& field);
157157
std::ostream& print(const Signature& sig);
158-
std::ostream& print(const Struct& struct_);
158+
std::ostream& print(const Struct& struct_,
159+
const std::unordered_map<Index, Name>& fieldNames);
159160
std::ostream& print(const Array& array);
160161
};
161162

@@ -1762,9 +1763,9 @@ std::ostream& TypePrinter::print(HeapType type) {
17621763
}
17631764
}
17641765

1765-
os << "(type ";
1766-
printHeapTypeName(type);
1767-
os << " ";
1766+
auto names = generator(type);
1767+
1768+
os << "(type $" << names.name << ' ';
17681769

17691770
if (isTemp(type)) {
17701771
os << "(; temp ;) ";
@@ -1787,7 +1788,7 @@ std::ostream& TypePrinter::print(HeapType type) {
17871788
if (type.isSignature()) {
17881789
print(type.getSignature());
17891790
} else if (type.isStruct()) {
1790-
print(type.getStruct());
1791+
print(type.getStruct(), names.fieldNames);
17911792
} else if (type.isArray()) {
17921793
print(type.getArray());
17931794
} else {
@@ -1854,19 +1855,24 @@ std::ostream& TypePrinter::print(const Signature& sig) {
18541855
return os << ')';
18551856
}
18561857

1857-
std::ostream& TypePrinter::print(const Struct& struct_) {
1858+
std::ostream&
1859+
TypePrinter::print(const Struct& struct_,
1860+
const std::unordered_map<Index, Name>& fieldNames) {
18581861
os << "(struct";
1859-
if (struct_.fields.size()) {
1860-
os << " (field";
1862+
for (Index i = 0; i < struct_.fields.size(); ++i) {
1863+
// TODO: move this to the function for printing fields.
1864+
os << " (field ";
1865+
if (auto it = fieldNames.find(i); it != fieldNames.end()) {
1866+
os << '$' << it->second << ' ';
1867+
}
1868+
print(struct_.fields[i]);
1869+
os << ')';
18611870
}
1862-
for (const Field& field : struct_.fields) {
1871+
// TODO: Remove this extra space kept to minimize test diffs.
1872+
if (struct_.fields.size() == 0) {
18631873
os << ' ';
1864-
print(field);
18651874
}
1866-
if (struct_.fields.size()) {
1867-
os << ')';
1868-
}
1869-
return os << ')';
1875+
return os << ")";
18701876
}
18711877

18721878
std::ostream& TypePrinter::print(const Array& array) {

test/atomics-unshared.wast.from-wast

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
(module
2-
(type $none_=>_none (func))
2+
(type $0 (func))
33
(memory $0 1 1)
4-
(func $foo (type $none_=>_none)
4+
(func $foo (type $0)
55
(drop
66
(i32.atomic.rmw.cmpxchg
77
(i32.const 0)

test/atomics-unshared.wast.fromBinary

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
(module
2-
(type $none_=>_none (func))
2+
(type $0 (func))
33
(memory $0 1 1)
4-
(func $foo (type $none_=>_none)
4+
(func $foo (type $0)
55
(drop
66
(i32.atomic.rmw.cmpxchg
77
(i32.const 0)

test/atomics-unshared.wast.fromBinary.noDebugInfo

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
(module
2-
(type $none_=>_none (func))
2+
(type $0 (func))
33
(memory $0 1 1)
4-
(func $0 (type $none_=>_none)
4+
(func $0 (type $0)
55
(drop
66
(i32.atomic.rmw.cmpxchg
77
(i32.const 0)

test/atomics.wast.fromBinary.noDebugInfo

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
(module
2-
(type $none_=>_none (func))
2+
(type $0 (func))
33
(memory $0 (shared 23 256))
4-
(func $0 (type $none_=>_none)
4+
(func $0 (type $0)
55
(local $0 i32)
66
(local $1 i64)
77
(drop
@@ -68,7 +68,7 @@
6868
(local.get $1)
6969
)
7070
)
71-
(func $1 (type $none_=>_none)
71+
(func $1 (type $0)
7272
(local $0 i32)
7373
(local $1 i64)
7474
(drop
@@ -102,7 +102,7 @@
102102
)
103103
)
104104
)
105-
(func $2 (type $none_=>_none)
105+
(func $2 (type $0)
106106
(local $0 i32)
107107
(local $1 i64)
108108
(drop
@@ -134,7 +134,7 @@
134134
)
135135
)
136136
)
137-
(func $3 (type $none_=>_none)
137+
(func $3 (type $0)
138138
(local $0 i32)
139139
(local $1 i64)
140140
(drop
@@ -178,7 +178,7 @@
178178
)
179179
)
180180
)
181-
(func $4 (type $none_=>_none)
181+
(func $4 (type $0)
182182
(atomic.fence)
183183
)
184184
)

test/atomics64.wast.fromBinary.noDebugInfo

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
(module
2-
(type $none_=>_none (func))
2+
(type $0 (func))
33
(memory $0 (shared i64 23 256))
4-
(func $0 (type $none_=>_none)
4+
(func $0 (type $0)
55
(local $0 i64)
66
(local $1 i64)
77
(local $2 i32)
@@ -69,7 +69,7 @@
6969
(local.get $1)
7070
)
7171
)
72-
(func $1 (type $none_=>_none)
72+
(func $1 (type $0)
7373
(local $0 i64)
7474
(local $1 i64)
7575
(local $2 i32)
@@ -104,7 +104,7 @@
104104
)
105105
)
106106
)
107-
(func $2 (type $none_=>_none)
107+
(func $2 (type $0)
108108
(local $0 i64)
109109
(local $1 i64)
110110
(local $2 i32)
@@ -137,7 +137,7 @@
137137
)
138138
)
139139
)
140-
(func $3 (type $none_=>_none)
140+
(func $3 (type $0)
141141
(local $0 i64)
142142
(local $1 i64)
143143
(local $2 i32)
@@ -182,7 +182,7 @@
182182
)
183183
)
184184
)
185-
(func $4 (type $none_=>_none)
185+
(func $4 (type $0)
186186
(atomic.fence)
187187
)
188188
)

test/binaryen.js/atomics.js.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
(module
2-
(type $none_=>_none (func))
2+
(type $0 (func))
33
(memory $0 (shared 1 1))
44
(func $main
55
(i32.atomic.store

test/binaryen.js/debug-info.js.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
=== default ===
22
debugInfo=false
33
(module
4-
(type $none_=>_none (func))
4+
(type $0 (func))
55
(memory $0 0)
66
(export "test" (func $0))
77
(func $0
@@ -23,7 +23,7 @@ debugInfo=true
2323
=== without debug info ===
2424
debugInfo=false
2525
(module
26-
(type $none_=>_none (func))
26+
(type $0 (func))
2727
(memory $0 0)
2828
(export "test" (func $0))
2929
(func $0

test/binaryen.js/debug-names.js.txt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
=== parsed wast ===
1212
(module $hel
13-
(type $i32_=>_none (func (param i32)))
13+
(type $0 (func (param i32)))
1414
(global $ld i32 (i32.const 0))
1515
(memory $lo 0 0)
1616
(table $wor 0 0 funcref)
@@ -22,7 +22,7 @@
2222

2323
=== roundtripped ===
2424
(module $hel
25-
(type $i32_=>_none (func (param i32)))
25+
(type $0 (func (param i32)))
2626
(global $ld i32 (i32.const 0))
2727
(memory $lo 0 0)
2828
(table $wor 0 0 funcref)
@@ -34,7 +34,7 @@
3434

3535
=== roundtripped again ===
3636
(module $hel
37-
(type $i32_=>_none (func (param i32)))
37+
(type $0 (func (param i32)))
3838
(global $ld i32 (i32.const 0))
3939
(memory $lo 0 0)
4040
(table $wor 0 0 funcref)

0 commit comments

Comments
 (0)