Skip to content

Implement bottom heap types #5115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
Oct 7, 2022
Merged

Implement bottom heap types #5115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
ab096d5
Implement bottom heap types
tlively Sep 7, 2022
b5b453a
nullopt
tlively Oct 5, 2022
d6c1a62
binaryen.js tests
tlively Oct 5, 2022
05ecd5f
Address feedback
tlively Oct 5, 2022
09d42f1
simplify some code
tlively Oct 5, 2022
328a068
Add tests
tlively Oct 5, 2022
ddfebfa
Changelog
tlively Oct 5, 2022
5b556ca
Replace ref.nulls with locals to avoid unreachable replacements in tests
tlively Oct 5, 2022
63e853e
update gufa test
tlively Oct 6, 2022
6c33c30
fix remove-unused-module-elements-refs test
tlively Oct 6, 2022
be4a1ff
remove stale optimize-instructions-tnh test
tlively Oct 6, 2022
f8c987b
fix merge-blocks test
tlively Oct 6, 2022
d4fd05c
fix dae-gc-refine-params.wast
tlively Oct 6, 2022
2310c41
Remove unfinished, stale comment
tlively Oct 6, 2022
93d3d7d
fix test/heap-types.wast
tlively Oct 6, 2022
d6a1fda
fix test/lit/passes/cfp.wast
tlively Oct 6, 2022
0d68f7d
Fix possible-contents handling of nulls
tlively Oct 6, 2022
6c4933e
restore and fix ArrayCopy effects test
tlively Oct 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ Current Trunk
- If `THROW_ON_FATAL` is defined at compile-time, then fatal errors will throw a
`std::runtime_error` instead of terminating the process. This may be used by
embedders of Binaryen to recover from errors.
- Implemented bottom heap types: `none`, `nofunc`, and `noextern`. RefNull
expressions and null `Literal`s must now have type `nullref`, `nullfuncref`,
or `nullexternref`.

v110
----
Expand Down
188 changes: 107 additions & 81 deletions src/binaryen-c.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,96 +51,111 @@ static_assert(sizeof(BinaryenLiteral) == sizeof(Literal),
BinaryenLiteral toBinaryenLiteral(Literal x) {
BinaryenLiteral ret;
ret.type = x.type.getID();
if (x.type.isRef()) {
auto heapType = x.type.getHeapType();
if (heapType.isBasic()) {
switch (heapType.getBasic()) {
case HeapType::func:
ret.func = x.isNull() ? nullptr : x.getFunc().c_str();
break;
case HeapType::ext:
case HeapType::eq:
assert(x.isNull() && "unexpected non-null reference type literal");
break;
case HeapType::any:
case HeapType::i31:
case HeapType::data:
case HeapType::string:
case HeapType::stringview_wtf8:
case HeapType::stringview_wtf16:
case HeapType::stringview_iter:
WASM_UNREACHABLE("TODO: reftypes");
}
return ret;
assert(x.type.isSingle());
if (x.type.isBasic()) {
switch (x.type.getBasic()) {
case Type::i32:
ret.i32 = x.geti32();
return ret;
case Type::i64:
ret.i64 = x.geti64();
return ret;
case Type::f32:
ret.i32 = x.reinterpreti32();
return ret;
case Type::f64:
ret.i64 = x.reinterpreti64();
return ret;
case Type::v128:
memcpy(&ret.v128, x.getv128Ptr(), 16);
return ret;
case Type::none:
case Type::unreachable:
WASM_UNREACHABLE("unexpected type");
}
WASM_UNREACHABLE("TODO: reftypes");
}
TODO_SINGLE_COMPOUND(x.type);
switch (x.type.getBasic()) {
case Type::i32:
ret.i32 = x.geti32();
break;
case Type::i64:
ret.i64 = x.geti64();
break;
case Type::f32:
ret.i32 = x.reinterpreti32();
break;
case Type::f64:
ret.i64 = x.reinterpreti64();
break;
case Type::v128:
memcpy(&ret.v128, x.getv128Ptr(), 16);
break;
case Type::none:
case Type::unreachable:
WASM_UNREACHABLE("unexpected type");
assert(x.type.isRef());
auto heapType = x.type.getHeapType();
if (heapType.isBasic()) {
switch (heapType.getBasic()) {
case HeapType::i31:
WASM_UNREACHABLE("TODO: i31");
case HeapType::ext:
case HeapType::any:
WASM_UNREACHABLE("TODO: extern literals");
case HeapType::eq:
case HeapType::func:
case HeapType::data:
WASM_UNREACHABLE("invalid type");
case HeapType::string:
case HeapType::stringview_wtf8:
case HeapType::stringview_wtf16:
case HeapType::stringview_iter:
WASM_UNREACHABLE("TODO: string literals");
case HeapType::none:
case HeapType::noext:
case HeapType::nofunc:
// Null.
return ret;
}
}
return ret;
if (heapType.isSignature()) {
ret.func = x.getFunc().c_str();
return ret;
}
assert(x.isData());
WASM_UNREACHABLE("TODO: gc data");
}

Literal fromBinaryenLiteral(BinaryenLiteral x) {
auto type = Type(x.type);
if (type.isRef()) {
auto heapType = type.getHeapType();
if (type.isNullable()) {
return Literal::makeNull(heapType);
if (type.isBasic()) {
switch (type.getBasic()) {
case Type::i32:
return Literal(x.i32);
case Type::i64:
return Literal(x.i64);
case Type::f32:
return Literal(x.i32).castToF32();
case Type::f64:
return Literal(x.i64).castToF64();
case Type::v128:
return Literal(x.v128);
case Type::none:
case Type::unreachable:
WASM_UNREACHABLE("unexpected type");
}
if (heapType.isBasic()) {
switch (heapType.getBasic()) {
case HeapType::func:
case HeapType::any:
case HeapType::eq:
case HeapType::data:
assert(false && "Literals must have concrete types");
WASM_UNREACHABLE("no fallthrough here");
case HeapType::ext:
case HeapType::i31:
case HeapType::string:
case HeapType::stringview_wtf8:
case HeapType::stringview_wtf16:
case HeapType::stringview_iter:
WASM_UNREACHABLE("TODO: reftypes");
}
}
assert(type.isRef());
auto heapType = type.getHeapType();
if (heapType.isBasic()) {
switch (heapType.getBasic()) {
case HeapType::i31:
WASM_UNREACHABLE("TODO: i31");
case HeapType::ext:
case HeapType::any:
WASM_UNREACHABLE("TODO: extern literals");
case HeapType::eq:
case HeapType::func:
case HeapType::data:
WASM_UNREACHABLE("invalid type");
case HeapType::string:
case HeapType::stringview_wtf8:
case HeapType::stringview_wtf16:
case HeapType::stringview_iter:
WASM_UNREACHABLE("TODO: string literals");
case HeapType::none:
case HeapType::noext:
case HeapType::nofunc:
assert(type.isNullable());
return Literal::makeNull(heapType);
}
}
assert(type.isBasic());
switch (type.getBasic()) {
case Type::i32:
return Literal(x.i32);
case Type::i64:
return Literal(x.i64);
case Type::f32:
return Literal(x.i32).castToF32();
case Type::f64:
return Literal(x.i64).castToF64();
case Type::v128:
return Literal(x.v128);
case Type::none:
case Type::unreachable:
WASM_UNREACHABLE("unexpected type");
if (heapType.isSignature()) {
return Literal::makeFunc(Name(x.func), heapType);
}
WASM_UNREACHABLE("invalid type");
assert(heapType.isData());
WASM_UNREACHABLE("TODO: gc data");
}

// Mutexes (global for now; in theory if multiple modules
Expand Down Expand Up @@ -197,6 +212,15 @@ BinaryenType BinaryenTypeStringviewWTF16() {
BinaryenType BinaryenTypeStringviewIter() {
return Type(HeapType::stringview_iter, Nullable).getID();
}
BinaryenType BinaryenTypeNullref() {
return Type(HeapType::none, Nullable).getID();
}
BinaryenType BinaryenTypeNullExternref(void) {
return Type(HeapType::noext, Nullable).getID();
}
BinaryenType BinaryenTypeNullFuncref(void) {
return Type(HeapType::nofunc, Nullable).getID();
}
BinaryenType BinaryenTypeUnreachable(void) { return Type::unreachable; }
BinaryenType BinaryenTypeAuto(void) { return uintptr_t(-1); }

Expand Down Expand Up @@ -1484,7 +1508,8 @@ BinaryenExpressionRef BinaryenRefNull(BinaryenModuleRef module,
BinaryenType type) {
Type type_(type);
assert(type_.isNullable());
return static_cast<Expression*>(Builder(*(Module*)module).makeRefNull(type_));
return static_cast<Expression*>(
Builder(*(Module*)module).makeRefNull(type_.getHeapType()));
}

BinaryenExpressionRef BinaryenRefIs(BinaryenModuleRef module,
Expand Down Expand Up @@ -1699,10 +1724,11 @@ BinaryenExpressionRef BinaryenArrayInit(BinaryenModuleRef module,
BinaryenExpressionRef BinaryenArrayGet(BinaryenModuleRef module,
BinaryenExpressionRef ref,
BinaryenExpressionRef index,
BinaryenType type,
bool signed_) {
return static_cast<Expression*>(
Builder(*(Module*)module)
.makeArrayGet((Expression*)ref, (Expression*)index, signed_));
.makeArrayGet((Expression*)ref, (Expression*)index, Type(type), signed_));
}
BinaryenExpressionRef BinaryenArraySet(BinaryenModuleRef module,
BinaryenExpressionRef ref,
Expand Down
4 changes: 4 additions & 0 deletions src/binaryen-c.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ BINARYEN_API BinaryenType BinaryenTypeStringref(void);
BINARYEN_API BinaryenType BinaryenTypeStringviewWTF8(void);
BINARYEN_API BinaryenType BinaryenTypeStringviewWTF16(void);
BINARYEN_API BinaryenType BinaryenTypeStringviewIter(void);
BINARYEN_API BinaryenType BinaryenTypeNullref(void);
BINARYEN_API BinaryenType BinaryenTypeNullExternref(void);
BINARYEN_API BinaryenType BinaryenTypeNullFuncref(void);
BINARYEN_API BinaryenType BinaryenTypeUnreachable(void);
// Not a real type. Used as the last parameter to BinaryenBlock to let
// the API figure out the type instead of providing one.
Expand Down Expand Up @@ -1044,6 +1047,7 @@ BinaryenArrayInit(BinaryenModuleRef module,
BINARYEN_API BinaryenExpressionRef BinaryenArrayGet(BinaryenModuleRef module,
BinaryenExpressionRef ref,
BinaryenExpressionRef index,
BinaryenType type,
bool signed_);
BINARYEN_API BinaryenExpressionRef
BinaryenArraySet(BinaryenModuleRef module,
Expand Down
28 changes: 28 additions & 0 deletions src/ir/effects.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,10 @@ class EffectAnalyzer {
}
}
void visitCallRef(CallRef* curr) {
if (curr->target->type.isNull()) {
parent.trap = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not necessary - we already set implicitTrap below. That could be reordered to the top, and then the early return would avoid other stuff if we need to avoid that.

I guess this does add more information in a sense, but optimizations should do this anyhow, I think?

Another concern I have here is how this interacts with trapsNeverHappen. We never set .trap directly so that we can gate on that flag.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you thinking that we should be setting implicitTrap when the target is null rather than setting trap? I think it's more consistent to set trap, just like visitUnreachable does. In particular, the CallRef will be emitted as (unreachable) in this case, so it would be strange if its effects were different from Unreachable's effects.

You're probably right that optimizations would take care of this anyway, but the code seems more correct to me as-is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking we can set implicitTrap when the target is nullable, which would include null. You're right that the current code is more precise in a way, but I don't think we gain from the extra verbosity.

I guess it's fine, though, if we make sure that it doesn't limit us. Specifically, with trapsNeverHappen and the new code then a callRef of a null would no longer be removed by vacuum (like it doesn't remove an unreachable). We should have a test that optimize-instructions turns the callRef into an unreachable (which dce would then remove). (There might already be a test, sorry if so - I haven't gotten to those yet.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems odd that trapsNeverHappen doesn't remove an unreachable, but I guess that's a separate topic.

Will make sure I add specific tests for the OptimizeInstructions changes.

return;
}
parent.calls = true;
if (parent.features.hasExceptionHandling() && parent.tryDepth == 0) {
parent.throws_ = true;
Expand All @@ -724,6 +728,10 @@ class EffectAnalyzer {
if (curr->ref->type == Type::unreachable) {
return;
}
if (curr->ref->type.isNull()) {
parent.trap = true;
return;
}
if (curr->ref->type.getHeapType()
.getStruct()
.fields[curr->index]
Expand All @@ -736,6 +744,10 @@ class EffectAnalyzer {
}
}
void visitStructSet(StructSet* curr) {
if (curr->ref->type.isNull()) {
parent.trap = true;
return;
}
parent.writesStruct = true;
// traps when the arg is null
if (curr->ref->type.isNullable()) {
Expand All @@ -745,22 +757,38 @@ class EffectAnalyzer {
void visitArrayNew(ArrayNew* curr) {}
void visitArrayInit(ArrayInit* curr) {}
void visitArrayGet(ArrayGet* curr) {
if (curr->ref->type.isNull()) {
parent.trap = true;
return;
}
parent.readsArray = true;
// traps when the arg is null or the index out of bounds
parent.implicitTrap = true;
}
void visitArraySet(ArraySet* curr) {
if (curr->ref->type.isNull()) {
parent.trap = true;
return;
}
parent.writesArray = true;
// traps when the arg is null or the index out of bounds
parent.implicitTrap = true;
}
void visitArrayLen(ArrayLen* curr) {
if (curr->ref->type.isNull()) {
parent.trap = true;
return;
}
// traps when the arg is null
if (curr->ref->type.isNullable()) {
parent.implicitTrap = true;
}
}
void visitArrayCopy(ArrayCopy* curr) {
if (curr->destRef->type.isNull() || curr->srcRef->type.isNull()) {
parent.trap = true;
return;
}
parent.readsArray = true;
parent.writesArray = true;
// traps when a ref is null, or when out of bounds.
Expand Down
1 change: 1 addition & 0 deletions src/ir/manipulation.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ template<typename InputType> inline Nop* nop(InputType* target) {

template<typename InputType>
inline RefNull* refNull(InputType* target, Type type) {
assert(type.isNullable() && type.getHeapType().isBottom());
auto* ret = convert<InputType, RefNull>(target);
ret->finalize(type);
return ret;
Expand Down
34 changes: 13 additions & 21 deletions src/ir/possible-contents.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,23 +47,6 @@ void PossibleContents::combine(const PossibleContents& other) {
// First handle the trivial cases of them being equal, or one of them is
// None or Many.
if (*this == other) {
// Nulls are a special case, since they compare equal even if their type is
// different. We would like to make this function symmetric, that is, that
// combine(a, b) == combine(b, a) (otherwise, things can be odd and we could
// get nondeterminism in the flow analysis which does not have a
// determinstic order). To fix that, pick the LUB.
if (isNull()) {
assert(other.isNull());
auto lub = HeapType::getLeastUpperBound(type.getHeapType(),
otherType.getHeapType());
if (!lub) {
// TODO: Remove this workaround once we have bottom types to assign to
// null literals.
value = Many();
return;
}
value = Literal::makeNull(*lub);
}
return;
}
if (other.isNone()) {
Expand Down Expand Up @@ -97,10 +80,18 @@ void PossibleContents::combine(const PossibleContents& other) {

// Special handling for references from here.

// Nulls are always equal to each other, even if their types differ.
if (isNull() && other.isNull()) {
// These must be nulls in different hierarchies, otherwise this would have
// been handled by the `*this == other` case above.
assert(type != otherType);
value = Many();
return;
}

// Nulls can be combined in by just adding nullability to a type.
if (isNull() || other.isNull()) {
// Only one of them can be null here, since we already checked if *this ==
// other, which would have been true had both been null.
// Only one of them can be null here, since we already handled the case
// where they were both null.
assert(!isNull() || !other.isNull());
// If only one is a null, but the other's type is known exactly, then the
// combination is to add nullability (if the type is *not* known exactly,
Expand Down Expand Up @@ -785,7 +776,8 @@ struct InfoCollector
// part of the main IR, which is potentially confusing during debugging,
// however, which is a downside.
Builder builder(*getModule());
auto* get = builder.makeArrayGet(curr->srcRef, curr->srcIndex);
auto* get =
builder.makeArrayGet(curr->srcRef, curr->srcIndex, curr->srcRef->type);
visitArrayGet(get);
auto* set = builder.makeArraySet(curr->destRef, curr->destIndex, get);
visitArraySet(set);
Expand Down
Loading