Skip to content

Commit 2d86d1f

Browse files
tlivelytlively
authored
Restore the extern heap type (#4898)
The GC proposal has split `any` and `extern` back into two separate types, so reintroduce `HeapType::ext` to represent `extern`. Before it was originally removed in #4633, externref was a subtype of anyref, but now it is not. Now that we have separate heaptype type hierarchies, make `HeapType::getLeastUpperBound` fallible as well.
1 parent 3aff4c6 commit 2d86d1f

Some content is hidden

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

43 files changed

+719
-606
lines changed

‎CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ Current Trunk
2222
argument `optimize`. (#4832)
2323
- Remove support for the `let` instruction that has been removed from the typed
2424
function references spec.
25+
- HeapType::ext has been restored but is no longer a subtype of HeapType::any to
26+
match the latest updates in the GC spec. (#4898)
2527

2628
v109
2729
----

‎src/binaryen-c.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,11 @@ BinaryenLiteral toBinaryenLiteral(Literal x) {
5858
case HeapType::func:
5959
ret.func = x.isNull() ? nullptr : x.getFunc().c_str();
6060
break;
61-
case HeapType::any:
61+
case HeapType::ext:
6262
case HeapType::eq:
6363
assert(x.isNull() && "unexpected non-null reference type literal");
6464
break;
65+
case HeapType::any:
6566
case HeapType::i31:
6667
case HeapType::data:
6768
case HeapType::string:
@@ -113,6 +114,7 @@ Literal fromBinaryenLiteral(BinaryenLiteral x) {
113114
case HeapType::data:
114115
assert(false && "Literals must have concrete types");
115116
WASM_UNREACHABLE("no fallthrough here");
117+
case HeapType::ext:
116118
case HeapType::i31:
117119
case HeapType::string:
118120
case HeapType::stringview_wtf8:
@@ -158,7 +160,6 @@ extern "C" {
158160
//
159161

160162
// Core types
161-
// TODO: Deprecate BinaryenTypeExternref?
162163

163164
BinaryenType BinaryenTypeNone(void) { return Type::none; }
164165
BinaryenType BinaryenTypeInt32(void) { return Type::i32; }
@@ -170,7 +171,7 @@ BinaryenType BinaryenTypeFuncref(void) {
170171
return Type(HeapType::func, Nullable).getID();
171172
}
172173
BinaryenType BinaryenTypeExternref(void) {
173-
return Type(HeapType::any, Nullable).getID();
174+
return Type(HeapType::ext, Nullable).getID();
174175
}
175176
BinaryenType BinaryenTypeAnyref(void) {
176177
return Type(HeapType::any, Nullable).getID();
@@ -239,6 +240,9 @@ BinaryenPackedType BinaryenPackedTypeInt16(void) {
239240

240241
// Heap types
241242

243+
BinaryenHeapType BinaryenHeapTypeExt() {
244+
return static_cast<BinaryenHeapType>(HeapType::BasicHeapType::ext);
245+
}
242246
BinaryenHeapType BinaryenHeapTypeFunc() {
243247
return static_cast<BinaryenHeapType>(HeapType::BasicHeapType::func);
244248
}

‎src/binaryen-c.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ BINARYEN_API BinaryenPackedType BinaryenPackedTypeInt16(void);
137137

138138
typedef uintptr_t BinaryenHeapType;
139139

140+
BINARYEN_API BinaryenHeapType BinaryenHeapTypeExt(void);
140141
BINARYEN_API BinaryenHeapType BinaryenHeapTypeFunc(void);
141142
BINARYEN_API BinaryenHeapType BinaryenHeapTypeAny(void);
142143
BINARYEN_API BinaryenHeapType BinaryenHeapTypeEq(void);

‎src/ir/possible-constant.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,14 @@ struct PossibleConstantValues {
114114
auto type = getConstantLiteral().type.getHeapType();
115115
auto otherType = other.getConstantLiteral().type.getHeapType();
116116
auto lub = HeapType::getLeastUpperBound(type, otherType);
117-
if (lub != type) {
118-
value = Literal::makeNull(lub);
117+
if (!lub) {
118+
// TODO: Remove this workaround once we have bottom types to assign to
119+
// null literals.
120+
value = Many();
121+
return true;
122+
}
123+
if (*lub != type) {
124+
value = Literal::makeNull(*lub);
119125
return true;
120126
}
121127
return false;

‎src/ir/possible-contents.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,13 @@ void PossibleContents::combine(const PossibleContents& other) {
5656
assert(other.isNull());
5757
auto lub = HeapType::getLeastUpperBound(type.getHeapType(),
5858
otherType.getHeapType());
59-
value = Literal::makeNull(lub);
59+
if (!lub) {
60+
// TODO: Remove this workaround once we have bottom types to assign to
61+
// null literals.
62+
value = Many();
63+
return;
64+
}
65+
value = Literal::makeNull(*lub);
6066
}
6167
return;
6268
}

‎src/passes/InstrumentLocals.cpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,15 +57,15 @@ Name get_f32("get_f32");
5757
Name get_f64("get_f64");
5858
Name get_v128("get_v128");
5959
Name get_funcref("get_funcref");
60-
Name get_anyref("get_anyref");
60+
Name get_externref("get_externref");
6161

6262
Name set_i32("set_i32");
6363
Name set_i64("set_i64");
6464
Name set_f32("set_f32");
6565
Name set_f64("set_f64");
6666
Name set_v128("set_v128");
6767
Name set_funcref("set_funcref");
68-
Name set_anyref("set_anyref");
68+
Name set_externref("set_externref");
6969

7070
struct InstrumentLocals : public WalkerPass<PostWalker<InstrumentLocals>> {
7171
void visitLocalGet(LocalGet* curr) {
@@ -75,8 +75,8 @@ struct InstrumentLocals : public WalkerPass<PostWalker<InstrumentLocals>> {
7575
auto heapType = curr->type.getHeapType();
7676
if (heapType == HeapType::func && curr->type.isNullable()) {
7777
import = get_funcref;
78-
} else if (heapType == HeapType::any && curr->type.isNullable()) {
79-
import = get_anyref;
78+
} else if (heapType == HeapType::ext && curr->type.isNullable()) {
79+
import = get_externref;
8080
} else {
8181
WASM_UNREACHABLE("TODO: general reference types");
8282
}
@@ -128,8 +128,8 @@ struct InstrumentLocals : public WalkerPass<PostWalker<InstrumentLocals>> {
128128
auto heapType = type.getHeapType();
129129
if (heapType == HeapType::func && type.isNullable()) {
130130
import = set_funcref;
131-
} else if (heapType == HeapType::any && type.isNullable()) {
132-
import = set_anyref;
131+
} else if (heapType == HeapType::ext && type.isNullable()) {
132+
import = set_externref;
133133
} else {
134134
WASM_UNREACHABLE("TODO: general reference types");
135135
}
@@ -175,12 +175,12 @@ struct InstrumentLocals : public WalkerPass<PostWalker<InstrumentLocals>> {
175175

176176
if (curr->features.hasReferenceTypes()) {
177177
Type func = Type(HeapType::func, Nullable);
178-
Type any = Type(HeapType::any, Nullable);
178+
Type ext = Type(HeapType::ext, Nullable);
179179

180180
addImport(curr, get_funcref, {Type::i32, Type::i32, func}, func);
181181
addImport(curr, set_funcref, {Type::i32, Type::i32, func}, func);
182-
addImport(curr, get_anyref, {Type::i32, Type::i32, any}, any);
183-
addImport(curr, set_anyref, {Type::i32, Type::i32, any}, any);
182+
addImport(curr, get_externref, {Type::i32, Type::i32, ext}, ext);
183+
addImport(curr, set_externref, {Type::i32, Type::i32, ext}, ext);
184184
}
185185
if (curr->features.hasSIMD()) {
186186
addImport(curr, get_v128, {Type::i32, Type::i32, Type::v128}, Type::v128);

‎src/passes/Print.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,9 @@ static bool maybePrintRefShorthand(std::ostream& o, Type type) {
8888
if (heapType.isBasic()) {
8989
if (type.isNullable()) {
9090
switch (heapType.getBasic()) {
91+
case HeapType::ext:
92+
o << "externref";
93+
return true;
9194
case HeapType::func:
9295
o << "funcref";
9396
return true;
@@ -115,6 +118,7 @@ static bool maybePrintRefShorthand(std::ostream& o, Type type) {
115118
}
116119
} else {
117120
switch (heapType.getBasic()) {
121+
case HeapType::ext:
118122
case HeapType::func:
119123
case HeapType::any:
120124
case HeapType::eq:

‎src/tools/fuzzing/fuzzing.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1968,6 +1968,10 @@ Expression* TranslateToFuzzReader::makeConstBasicRef(Type type) {
19681968
assert(heapType.isBasic());
19691969
assert(wasm.features.hasReferenceTypes());
19701970
switch (heapType.getBasic()) {
1971+
case HeapType::ext: {
1972+
assert(type.isNullable() && "Cannot handle non-nullable externref");
1973+
return builder.makeRefNull(type);
1974+
}
19711975
case HeapType::func: {
19721976
return makeRefFuncConst(type);
19731977
}
@@ -3089,13 +3093,14 @@ HeapType TranslateToFuzzReader::getSubType(HeapType type) {
30893093
case HeapType::func:
30903094
// TODO: Typed function references.
30913095
return HeapType::func;
3096+
case HeapType::ext:
3097+
return HeapType::ext;
30923098
case HeapType::any:
30933099
// TODO: nontrivial types as well.
30943100
return pick(
30953101
FeatureOptions<HeapType>()
3096-
.add(FeatureSet::ReferenceTypes, HeapType::func, HeapType::any)
3102+
.add(FeatureSet::ReferenceTypes, HeapType::func /*, HeapType::ext*/)
30973103
.add(FeatureSet::ReferenceTypes | FeatureSet::GC,
3098-
HeapType::func,
30993104
HeapType::any,
31003105
HeapType::eq,
31013106
HeapType::i31,

‎src/tools/fuzzing/heap-types.cpp

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ struct HeapTypeGeneratorImpl {
153153

154154
HeapType::BasicHeapType generateBasicHeapType() {
155155
return rand.pick(HeapType::func,
156+
HeapType::ext,
156157
HeapType::any,
157158
HeapType::eq,
158159
HeapType::i31,
@@ -278,6 +279,7 @@ struct HeapTypeGeneratorImpl {
278279
return type;
279280
} else {
280281
switch (type) {
282+
case HeapType::ext:
281283
case HeapType::i31:
282284
// No other subtypes.
283285
return type;
@@ -371,6 +373,8 @@ struct HeapTypeGeneratorImpl {
371373
// This is not a constructed type, so it must be a basic type.
372374
assert(type.isBasic());
373375
switch (type.getBasic()) {
376+
case HeapType::ext:
377+
return HeapType::ext;
374378
case HeapType::func:
375379
return pickSubFunc();
376380
case HeapType::any:
@@ -477,16 +481,31 @@ struct HeapTypeGeneratorImpl {
477481
switch (*basic) {
478482
case HeapType::func:
479483
return SignatureKind{};
484+
case HeapType::ext:
480485
case HeapType::i31:
481486
return super;
482487
case HeapType::any:
483-
return generateHeapTypeKind();
488+
if (rand.oneIn(4)) {
489+
switch (rand.upTo(3)) {
490+
case 0:
491+
return HeapType::eq;
492+
case 1:
493+
return HeapType::i31;
494+
case 2:
495+
return HeapType::data;
496+
}
497+
}
498+
return DataKind{};
484499
case HeapType::eq:
485500
if (rand.oneIn(4)) {
486-
return HeapType::i31;
487-
} else {
488-
return DataKind{};
501+
switch (rand.upTo(2)) {
502+
case 0:
503+
return HeapType::i31;
504+
case 1:
505+
return HeapType::data;
506+
}
489507
}
508+
return DataKind{};
490509
case HeapType::data:
491510
return DataKind{};
492511
case HeapType::string:

‎src/tools/wasm-fuzz-types.cpp

Lines changed: 66 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -152,46 +152,72 @@ void Fuzzer::checkLUBs() const {
152152
HeapType a = types[i], b = types[j];
153153
// Check that their LUB is stable when calculated multiple times and in
154154
// reverse order.
155-
HeapType lub = HeapType::getLeastUpperBound(a, b);
156-
if (lub != HeapType::getLeastUpperBound(b, a) ||
157-
lub != HeapType::getLeastUpperBound(a, b)) {
158-
Fatal() << "Could not calculate a stable LUB of HeapTypes " << i
159-
<< " and " << j << "!\n"
160-
<< i << ": " << a << "\n"
161-
<< j << ": " << b << "\n";
162-
}
163-
// Check that each type is a subtype of the LUB.
164-
if (!HeapType::isSubType(a, lub)) {
165-
Fatal() << "HeapType " << i
166-
<< " is not a subtype of its LUB with HeapType " << j << "!\n"
167-
<< i << ": " << a << "\n"
168-
<< j << ": " << b << "\n"
169-
<< "lub: " << lub << "\n";
170-
}
171-
if (!HeapType::isSubType(b, lub)) {
172-
Fatal() << "HeapType " << j
173-
<< " is not a subtype of its LUB with HeapType " << i << "!\n"
174-
<< i << ": " << a << "\n"
175-
<< j << ": " << b << "\n"
176-
<< "lub: " << lub << "\n";
177-
}
178-
// Check that the LUB of each type and the original LUB is still the
179-
// original LUB.
180-
if (lub != HeapType::getLeastUpperBound(a, lub)) {
181-
Fatal() << "The LUB of HeapType " << i << " and HeapType " << j
182-
<< " should be the LUB of itself and HeapType " << i
183-
<< ", but it is not!\n"
184-
<< i << ": " << a << "\n"
185-
<< j << ": " << b << "\n"
186-
<< "lub: " << lub << "\n";
187-
}
188-
if (lub != HeapType::getLeastUpperBound(lub, b)) {
189-
Fatal() << "The LUB of HeapType " << i << " and HeapType " << j
190-
<< " should be the LUB of itself and HeapType " << j
191-
<< ", but it is not!\n"
192-
<< i << ": " << a << "\n"
193-
<< j << ": " << b << "\n"
194-
<< "lub: " << lub << "\n";
155+
auto lub = HeapType::getLeastUpperBound(a, b);
156+
if (lub) {
157+
if (lub != HeapType::getLeastUpperBound(b, a) ||
158+
lub != HeapType::getLeastUpperBound(a, b)) {
159+
Fatal() << "Could not calculate a stable LUB of HeapTypes " << i
160+
<< " and " << j << "!\n"
161+
<< i << ": " << a << "\n"
162+
<< j << ": " << b << "\n";
163+
}
164+
// Check that each type is a subtype of the LUB.
165+
if (!HeapType::isSubType(a, *lub)) {
166+
Fatal() << "HeapType " << i
167+
<< " is not a subtype of its LUB with HeapType " << j << "!\n"
168+
<< i << ": " << a << "\n"
169+
<< j << ": " << b << "\n"
170+
<< "lub: " << *lub << "\n";
171+
}
172+
if (!HeapType::isSubType(b, *lub)) {
173+
Fatal() << "HeapType " << j
174+
<< " is not a subtype of its LUB with HeapType " << i << "!\n"
175+
<< i << ": " << a << "\n"
176+
<< j << ": " << b << "\n"
177+
<< "lub: " << *lub << "\n";
178+
}
179+
// Check that the LUB of each type and the original LUB is still the
180+
// original LUB.
181+
if (lub != HeapType::getLeastUpperBound(a, *lub)) {
182+
Fatal() << "The LUB of HeapType " << i << " and HeapType " << j
183+
<< " should be the LUB of itself and HeapType " << i
184+
<< ", but it is not!\n"
185+
<< i << ": " << a << "\n"
186+
<< j << ": " << b << "\n"
187+
<< "lub: " << *lub << "\n";
188+
}
189+
if (lub != HeapType::getLeastUpperBound(*lub, b)) {
190+
Fatal() << "The LUB of HeapType " << i << " and HeapType " << j
191+
<< " should be the LUB of itself and HeapType " << j
192+
<< ", but it is not!\n"
193+
<< i << ": " << a << "\n"
194+
<< j << ": " << b << "\n"
195+
<< "lub: " << *lub << "\n";
196+
}
197+
} else {
198+
// No LUB. Check that this is symmetrical.
199+
if (auto lub2 = HeapType::getLeastUpperBound(b, a)) {
200+
Fatal() << "There is no LUB of HeapType " << i << " and HeapType "
201+
<< j << ", but there is a LUB in the reverse direction!\n"
202+
<< i << ": " << a << "\n"
203+
<< j << ": " << b << "\n"
204+
<< "lub: " << *lub2 << "\n";
205+
}
206+
// There also shouldn't be a subtype relation in this case.
207+
if (HeapType::isSubType(a, b)) {
208+
Fatal() << "There is no LUB of HeapType " << i << " and HeapType "
209+
<< j << ", but HeapType " << i << " is a subtype of HeapType "
210+
<< j << "!\n"
211+
<< i << ": " << a << "\n"
212+
<< j << ": " << b << "\n";
213+
}
214+
if (HeapType::isSubType(b, a)) {
215+
Fatal() << "There is no LUB of HeapType " << i << " and HeapType "
216+
<< j << ", but HeapType " << j << " is a subtype of HeapType "
217+
<< i << "!\n"
218+
<< i << ": " << a << "\n"
219+
<< j << ": " << b << "\n";
220+
}
195221
}
196222
}
197223
}

0 commit comments

Comments
 (0)