Skip to content

Commit 21a915f

Browse files
kripkenkripken
authored
[CustomDescriptors] TypeUpdating: Sort descriptors properly when sorting types (#7649)
The spec constrains the order of described/descriptor type pairs.
1 parent ba0c818 commit 21a915f

File tree

3 files changed

+184
-43
lines changed

3 files changed

+184
-43
lines changed

‎scripts/test/fuzzing.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@
118118
'ref.get_desc.wast',
119119
'ref.cast_desc.wast',
120120
'struct.new-desc.wast',
121+
'remove-unused-types-descriptors.wast',
121122
# TODO: fix split_wast() on tricky escaping situations like a string ending
122123
# in \\" (the " is not escaped - there is an escaped \ before it)
123124
'string-lifting-section.wast',

‎src/ir/type-updating.cpp

Lines changed: 57 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -48,59 +48,73 @@ GlobalTypeRewriter::TypeMap GlobalTypeRewriter::rebuildTypes(
4848
std::unordered_set<HeapType> additionalSet(additionalPrivateTypes.begin(),
4949
additionalPrivateTypes.end());
5050

51-
std::vector<std::pair<HeapType, SmallVector<HeapType, 1>>> privateSupertypes;
52-
privateSupertypes.reserve(typeInfo.size());
51+
// Check if a type is private, given the info for it.
52+
auto isPublicGivenInfo = [&](HeapType type, auto& info) {
53+
return info.visibility != ModuleUtils::Visibility::Private &&
54+
!additionalSet.count(type);
55+
};
56+
57+
// Check if a type is private, looking for its info (if there is none, it is
58+
// not private).
59+
auto isPublic = [&](HeapType type) {
60+
auto it = typeInfo.find(type);
61+
if (it == typeInfo.end()) {
62+
return false;
63+
}
64+
return isPublicGivenInfo(type, it->second);
65+
};
66+
67+
// For each type, note all the predecessors it must have, i.e., that must
68+
// appear before it. That includes supertypes and described types.
69+
std::vector<std::pair<HeapType, SmallVector<HeapType, 1>>> privatePreds;
70+
privatePreds.reserve(typeInfo.size());
5371
for (auto& [type, info] : typeInfo) {
54-
if (info.visibility != ModuleUtils::Visibility::Private &&
55-
!additionalSet.count(type)) {
72+
if (isPublicGivenInfo(type, info)) {
5673
continue;
5774
}
58-
privateSupertypes.push_back({type, {}});
75+
privatePreds.push_back({type, {}});
5976

60-
if (auto super = getDeclaredSuperType(type)) {
61-
auto it = typeInfo.find(*super);
62-
// Record the supertype only if it is among the private types.
63-
if ((it != typeInfo.end() &&
64-
it->second.visibility == ModuleUtils::Visibility::Private) ||
65-
additionalSet.count(*super)) {
66-
privateSupertypes.back().second.push_back(*super);
67-
}
77+
// Check for a (private) supertype.
78+
if (auto super = getDeclaredSuperType(type); super && !isPublic(*super)) {
79+
privatePreds.back().second.push_back(*super);
80+
}
81+
82+
// Check for a (private) described type.
83+
if (auto desc = type.getDescribedType()) {
84+
// It is not possible for a a described type to be public while its
85+
// descriptor is private, or vice versa.
86+
assert(!isPublic(*desc));
87+
privatePreds.back().second.push_back(*desc);
6888
}
6989
}
7090

71-
// Topological sort to have subtypes first. This is the opposite of the
72-
// order we need, so the comparison is the opposite of what we ultimately
73-
// want.
7491
std::vector<HeapType> sorted;
7592
if (wasm.typeIndices.empty()) {
76-
sorted = TopologicalSort::sortOf(privateSupertypes.begin(),
77-
privateSupertypes.end());
93+
sorted = TopologicalSort::sortOf(privatePreds.begin(), privatePreds.end());
7894
} else {
79-
sorted =
80-
TopologicalSort::minSortOf(privateSupertypes.begin(),
81-
privateSupertypes.end(),
82-
[&](Index a, Index b) {
83-
auto typeA = privateSupertypes[a].first;
84-
auto typeB = privateSupertypes[b].first;
85-
// Preserve type order.
86-
auto itA = wasm.typeIndices.find(typeA);
87-
auto itB = wasm.typeIndices.find(typeB);
88-
bool hasA = itA != wasm.typeIndices.end();
89-
bool hasB = itB != wasm.typeIndices.end();
90-
if (hasA != hasB) {
91-
// Types with preserved indices must be
92-
// sorted before (after in this reversed
93-
// comparison) types without indices to
94-
// maintain transitivity.
95-
return !hasA;
96-
}
97-
if (hasA && *itA != *itB) {
98-
return !(itA->second < itB->second);
99-
}
100-
// Break ties by the arbitrary order we
101-
// have collected the types in.
102-
return a > b;
103-
});
95+
sorted = TopologicalSort::minSortOf(
96+
privatePreds.begin(), privatePreds.end(), [&](Index a, Index b) {
97+
auto typeA = privatePreds[a].first;
98+
auto typeB = privatePreds[b].first;
99+
// Preserve type order.
100+
auto itA = wasm.typeIndices.find(typeA);
101+
auto itB = wasm.typeIndices.find(typeB);
102+
bool hasA = itA != wasm.typeIndices.end();
103+
bool hasB = itB != wasm.typeIndices.end();
104+
if (hasA != hasB) {
105+
// Types with preserved indices must be
106+
// sorted before (after in this reversed
107+
// comparison) types without indices to
108+
// maintain transitivity.
109+
return !hasA;
110+
}
111+
if (hasA && *itA != *itB) {
112+
return !(itA->second < itB->second);
113+
}
114+
// Break ties by the arbitrary order we
115+
// have collected the types in.
116+
return a > b;
117+
});
104118
}
105119
std::reverse(sorted.begin(), sorted.end());
106120
Index i = 0;

‎test/lit/passes/remove-unused-types-descriptors.wast

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
2+
3+
;; RUN: foreach %s %t wasm-opt --remove-unused-types --closed-world -all -S -o - | filecheck %s
4+
5+
;; Use described the most, middle next, and describing least.
6+
;; The unused type lets remove-unused-types work and sort the types, so we
7+
;; can test the sort is valid.
8+
(module
9+
(rec
10+
;; CHECK: (rec
11+
;; CHECK-NEXT: (type $described (descriptor $middle (struct)))
12+
(type $described (descriptor $middle (struct)))
13+
;; CHECK: (type $middle (describes $described (descriptor $describing (struct))))
14+
(type $middle (describes $described (descriptor $describing (struct))))
15+
;; CHECK: (type $describing (describes $middle (struct)))
16+
(type $describing (describes $middle (struct)))
17+
18+
(type $unused (struct))
19+
)
20+
21+
;; CHECK: (type $3 (func (param anyref)))
22+
23+
;; CHECK: (func $uses (type $3) (param $x anyref)
24+
;; CHECK-NEXT: (drop
25+
;; CHECK-NEXT: (ref.cast (ref $described)
26+
;; CHECK-NEXT: (local.get $x)
27+
;; CHECK-NEXT: )
28+
;; CHECK-NEXT: )
29+
;; CHECK-NEXT: (drop
30+
;; CHECK-NEXT: (ref.cast (ref $described)
31+
;; CHECK-NEXT: (local.get $x)
32+
;; CHECK-NEXT: )
33+
;; CHECK-NEXT: )
34+
;; CHECK-NEXT: (drop
35+
;; CHECK-NEXT: (ref.cast (ref $described)
36+
;; CHECK-NEXT: (local.get $x)
37+
;; CHECK-NEXT: )
38+
;; CHECK-NEXT: )
39+
;; CHECK-NEXT: (drop
40+
;; CHECK-NEXT: (ref.cast (ref $middle)
41+
;; CHECK-NEXT: (local.get $x)
42+
;; CHECK-NEXT: )
43+
;; CHECK-NEXT: )
44+
;; CHECK-NEXT: (drop
45+
;; CHECK-NEXT: (ref.cast (ref $middle)
46+
;; CHECK-NEXT: (local.get $x)
47+
;; CHECK-NEXT: )
48+
;; CHECK-NEXT: )
49+
;; CHECK-NEXT: (drop
50+
;; CHECK-NEXT: (ref.cast (ref $describing)
51+
;; CHECK-NEXT: (local.get $x)
52+
;; CHECK-NEXT: )
53+
;; CHECK-NEXT: )
54+
;; CHECK-NEXT: )
55+
(func $uses (param $x anyref)
56+
(drop (ref.cast (ref $described) (local.get $x)))
57+
(drop (ref.cast (ref $described) (local.get $x)))
58+
(drop (ref.cast (ref $described) (local.get $x)))
59+
60+
(drop (ref.cast (ref $middle) (local.get $x)))
61+
(drop (ref.cast (ref $middle) (local.get $x)))
62+
63+
(drop (ref.cast (ref $describing) (local.get $x)))
64+
)
65+
)
66+
67+
;; As above, but with use counts flipped.
68+
(module
69+
(rec
70+
;; CHECK: (rec
71+
;; CHECK-NEXT: (type $described (descriptor $middle (struct)))
72+
(type $described (descriptor $middle (struct)))
73+
;; CHECK: (type $middle (describes $described (descriptor $describing (struct))))
74+
(type $middle (describes $described (descriptor $describing (struct))))
75+
;; CHECK: (type $describing (describes $middle (struct)))
76+
(type $describing (describes $middle (struct)))
77+
78+
(type $unused (struct))
79+
)
80+
81+
;; CHECK: (type $3 (func (param anyref)))
82+
83+
;; CHECK: (func $uses (type $3) (param $x anyref)
84+
;; CHECK-NEXT: (drop
85+
;; CHECK-NEXT: (ref.cast (ref $described)
86+
;; CHECK-NEXT: (local.get $x)
87+
;; CHECK-NEXT: )
88+
;; CHECK-NEXT: )
89+
;; CHECK-NEXT: (drop
90+
;; CHECK-NEXT: (ref.cast (ref $middle)
91+
;; CHECK-NEXT: (local.get $x)
92+
;; CHECK-NEXT: )
93+
;; CHECK-NEXT: )
94+
;; CHECK-NEXT: (drop
95+
;; CHECK-NEXT: (ref.cast (ref $middle)
96+
;; CHECK-NEXT: (local.get $x)
97+
;; CHECK-NEXT: )
98+
;; CHECK-NEXT: )
99+
;; CHECK-NEXT: (drop
100+
;; CHECK-NEXT: (ref.cast (ref $describing)
101+
;; CHECK-NEXT: (local.get $x)
102+
;; CHECK-NEXT: )
103+
;; CHECK-NEXT: )
104+
;; CHECK-NEXT: (drop
105+
;; CHECK-NEXT: (ref.cast (ref $describing)
106+
;; CHECK-NEXT: (local.get $x)
107+
;; CHECK-NEXT: )
108+
;; CHECK-NEXT: )
109+
;; CHECK-NEXT: (drop
110+
;; CHECK-NEXT: (ref.cast (ref $describing)
111+
;; CHECK-NEXT: (local.get $x)
112+
;; CHECK-NEXT: )
113+
;; CHECK-NEXT: )
114+
;; CHECK-NEXT: )
115+
(func $uses (param $x anyref)
116+
(drop (ref.cast (ref $described) (local.get $x)))
117+
118+
(drop (ref.cast (ref $middle) (local.get $x)))
119+
(drop (ref.cast (ref $middle) (local.get $x)))
120+
121+
(drop (ref.cast (ref $describing) (local.get $x)))
122+
(drop (ref.cast (ref $describing) (local.get $x)))
123+
(drop (ref.cast (ref $describing) (local.get $x)))
124+
)
125+
)
126+

0 commit comments

Comments
 (0)