Skip to content

Commit

Permalink
Rename external conversion instructions (#6716)
Browse files Browse the repository at this point in the history
Rename instructions `extern.internalize` into `any.convert_extern` and
`extern.externalize` into `extern.convert_any` to follow more closely
the spec. This was changed in
WebAssembly/gc#432.

The legacy name is still accepted in text inputs and in the C and JS
APIs.
  • Loading branch information
vouillon committed Jul 8, 2024
1 parent 9792f2c commit 81f8f77
Show file tree
Hide file tree
Showing 37 changed files with 161 additions and 122 deletions.
2 changes: 1 addition & 1 deletion scripts/clang-tidy-diff.sh
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ function realpath() {

CLANG_DIR=$(dirname $(dirname $(realpath $CLANG_TIDY)))
CLANG_TIDY_DIFF=$CLANG_DIR/share/clang/clang-tidy-diff.py
ARG="-quiet -p1 -iregex=src/.*\.(cpp|cc|c\+\+|cxx|c|cl|h|hpp|m|mm|inc)"
ARG="-quiet -p1 -iregex=src/.*\.(cpp|cc|c\+\+|cxx|c|cl|h|hpp|m|mm)"
if [ ! -e "$CLANG_TIDY_DIFF" ]; then
echo "Failed to find clang-tidy-diff.py ($CLANG_TIDY_DIFF)"
exit 1
Expand Down
6 changes: 4 additions & 2 deletions scripts/gen-s-parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -603,8 +603,10 @@
("array.init_data", "makeArrayInitData()"),
("array.init_elem", "makeArrayInitElem()"),
("ref.as_non_null", "makeRefAs(RefAsNonNull)"),
("extern.internalize", "makeRefAs(ExternInternalize)"),
("extern.externalize", "makeRefAs(ExternExternalize)"),
("extern.internalize", "makeRefAs(AnyConvertExtern)"), # Deprecated
("extern.externalize", "makeRefAs(ExternConvertAny)"), # Deprecated
("any.convert_extern", "makeRefAs(AnyConvertExtern)"),
("extern.convert_any", "makeRefAs(ExternConvertAny)"),
("string.new_lossy_utf8_array", "makeStringNew(StringNewLossyUTF8Array)"),
("string.new_wtf16_array", "makeStringNew(StringNewWTF16Array)"),
("string.from_code_point", "makeStringNew(StringNewFromCodePoint)"),
Expand Down
6 changes: 4 additions & 2 deletions src/binaryen-c.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1012,8 +1012,10 @@ BinaryenOp BinaryenDotI8x16I7x16SToVecI16x8(void) {
return DotI8x16I7x16SToVecI16x8;
}
BinaryenOp BinaryenRefAsNonNull(void) { return RefAsNonNull; }
BinaryenOp BinaryenRefAsExternInternalize(void) { return ExternInternalize; }
BinaryenOp BinaryenRefAsExternExternalize(void) { return ExternExternalize; }
BinaryenOp BinaryenRefAsExternInternalize(void) { return AnyConvertExtern; }
BinaryenOp BinaryenRefAsExternExternalize(void) { return ExternConvertAny; }
BinaryenOp BinaryenRefAsAnyConvertExtern(void) { return AnyConvertExtern; }
BinaryenOp BinaryenRefAsExternConvertAny(void) { return ExternConvertAny; }
BinaryenOp BinaryenBrOnNull(void) { return BrOnNull; }
BinaryenOp BinaryenBrOnNonNull(void) { return BrOnNonNull; }
BinaryenOp BinaryenBrOnCast(void) { return BrOnCast; }
Expand Down
2 changes: 2 additions & 0 deletions src/binaryen-c.h
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,8 @@ BINARYEN_API BinaryenOp BinaryenDotI8x16I7x16SToVecI16x8(void);
BINARYEN_API BinaryenOp BinaryenRefAsNonNull(void);
BINARYEN_API BinaryenOp BinaryenRefAsExternInternalize(void);
BINARYEN_API BinaryenOp BinaryenRefAsExternExternalize(void);
BINARYEN_API BinaryenOp BinaryenRefAsAnyConvertExtern(void);
BINARYEN_API BinaryenOp BinaryenRefAsExternConvertAny(void);
BINARYEN_API BinaryenOp BinaryenBrOnNull(void);
BINARYEN_API BinaryenOp BinaryenBrOnNonNull(void);
BINARYEN_API BinaryenOp BinaryenBrOnCast(void);
Expand Down
16 changes: 14 additions & 2 deletions src/gen-s-parser.inc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ memcpy(buf, op.data(), op.size());
switch (buf[0]) {
case 'a': {
switch (buf[1]) {
case 'n':
if (op == "any.convert_extern"sv) {
CHECK_ERR(makeRefAs(ctx, pos, annotations, AnyConvertExtern));
return Ok{};
}
goto parse_error;
case 'r': {
switch (buf[6]) {
case 'c':
Expand Down Expand Up @@ -278,15 +284,21 @@ switch (buf[0]) {
}
case 'e': {
switch (buf[7]) {
case 'c':
if (op == "extern.convert_any"sv) {
CHECK_ERR(makeRefAs(ctx, pos, annotations, ExternConvertAny));
return Ok{};
}
goto parse_error;
case 'e':
if (op == "extern.externalize"sv) {
CHECK_ERR(makeRefAs(ctx, pos, annotations, ExternExternalize));
CHECK_ERR(makeRefAs(ctx, pos, annotations, ExternConvertAny));
return Ok{};
}
goto parse_error;
case 'i':
if (op == "extern.internalize"sv) {
CHECK_ERR(makeRefAs(ctx, pos, annotations, ExternInternalize));
CHECK_ERR(makeRefAs(ctx, pos, annotations, AnyConvertExtern));
return Ok{};
}
goto parse_error;
Expand Down
4 changes: 2 additions & 2 deletions src/ir/child-typer.h
Original file line number Diff line number Diff line change
Expand Up @@ -949,10 +949,10 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
case RefAsNonNull:
noteAnyReference(&curr->value);
return;
case ExternInternalize:
case AnyConvertExtern:
note(&curr->value, Type(HeapType::ext, Nullable));
return;
case ExternExternalize:
case ExternConvertAny:
note(&curr->value, Type(HeapType::any, Nullable));
return;
}
Expand Down
2 changes: 1 addition & 1 deletion src/ir/effects.h
Original file line number Diff line number Diff line change
Expand Up @@ -926,7 +926,7 @@ class EffectAnalyzer {
void visitArrayInitData(ArrayInitData* curr) { visitArrayInit(curr); }
void visitArrayInitElem(ArrayInitElem* curr) { visitArrayInit(curr); }
void visitRefAs(RefAs* curr) {
if (curr->op == ExternInternalize || curr->op == ExternExternalize) {
if (curr->op == AnyConvertExtern || curr->op == ExternConvertAny) {
// These conversions are infallible.
return;
}
Expand Down
2 changes: 1 addition & 1 deletion src/ir/possible-contents.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,7 @@ struct InfoCollector
receiveChildValue(curr->ref, curr);
}
void visitRefAs(RefAs* curr) {
if (curr->op == ExternExternalize || curr->op == ExternInternalize) {
if (curr->op == ExternConvertAny || curr->op == AnyConvertExtern) {
// The external conversion ops emit something of a completely different
// type, which we must mark as a root.
addRoot(curr);
Expand Down
2 changes: 1 addition & 1 deletion src/ir/properties.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ static bool isValidInConstantExpression(Module& wasm, Expression* expr) {
}

if (auto* refAs = expr->dynCast<RefAs>()) {
if (refAs->op == ExternExternalize || refAs->op == ExternInternalize) {
if (refAs->op == ExternConvertAny || refAs->op == AnyConvertExtern) {
return true;
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/ir/properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ inline bool isNamedControlFlow(Expression* curr) {
// isValidInConstantExpression or find better names(#4845)
inline bool isSingleConstantExpression(const Expression* curr) {
if (auto* refAs = curr->dynCast<RefAs>()) {
if (refAs->op == ExternExternalize || refAs->op == ExternInternalize) {
if (refAs->op == ExternConvertAny || refAs->op == AnyConvertExtern) {
return isSingleConstantExpression(refAs->value);
}
}
Expand Down Expand Up @@ -124,9 +124,9 @@ inline Literal getLiteral(const Expression* curr) {
} else if (auto* s = curr->dynCast<StringConst>()) {
return Literal(s->string.toString());
} else if (auto* r = curr->dynCast<RefAs>()) {
if (r->op == ExternExternalize) {
if (r->op == ExternConvertAny) {
return getLiteral(r->value).externalize();
} else if (r->op == ExternInternalize) {
} else if (r->op == AnyConvertExtern) {
return getLiteral(r->value).internalize();
}
}
Expand Down Expand Up @@ -329,7 +329,7 @@ inline Expression** getImmediateFallthroughPtr(
// Extern conversions are not casts and actually produce new values.
// Treating them as fallthroughs would lead to misoptimizations of
// subsequent casts.
if (as->op != ExternInternalize && as->op != ExternExternalize) {
if (as->op != AnyConvertExtern && as->op != ExternConvertAny) {
return &as->value;
}
} else if (auto* br = curr->dynCast<BrOn>()) {
Expand Down
6 changes: 4 additions & 2 deletions src/js/binaryen.js-post.js
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,8 @@ function initializeConstants() {
'RefAsNonNull',
'RefAsExternInternalize',
'RefAsExternExternalize',
'RefAsAnyConvertExtern',
'RefAsExternConvertAny',
'BrOnNull',
'BrOnNonNull',
'BrOnCast',
Expand Down Expand Up @@ -2380,8 +2382,8 @@ function wrapModule(module, self = {}) {
}
};

// TODO: extern.internalize
// TODO: extern.externalize
// TODO: any.convert_extern
// TODO: extern.convert_any
// TODO: ref.test
// TODO: ref.cast
// TODO: br_on_*
Expand Down
2 changes: 1 addition & 1 deletion src/passes/OptimizeCasts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
// )
//
// Note that right now, we only consider RefAs with op RefAsNonNull as a cast.
// RefAs with ExternInternalize and ExternExternalize are not considered casts
// RefAs with AnyConvertExtern and ExternConvertAny are not considered casts
// when obtaining fallthroughs, and so are ignored.
//
// TODO: Look past individual basic blocks? This may be worth considering
Expand Down
2 changes: 1 addition & 1 deletion src/passes/OptimizeInstructions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2440,7 +2440,7 @@ struct OptimizeInstructions
return;
}

if (curr->op == ExternExternalize || curr->op == ExternInternalize) {
if (curr->op == ExternConvertAny || curr->op == AnyConvertExtern) {
// We can't optimize these. Even removing a non-null cast is not valid as
// they allow nulls to filter through, unlike other RefAs*.
return;
Expand Down
8 changes: 4 additions & 4 deletions src/passes/Print.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2236,11 +2236,11 @@ struct PrintExpressionContents
case RefAsNonNull:
printMedium(o, "ref.as_non_null");
break;
case ExternInternalize:
printMedium(o, "extern.internalize");
case AnyConvertExtern:
printMedium(o, "any.convert_extern");
break;
case ExternExternalize:
printMedium(o, "extern.externalize");
case ExternConvertAny:
printMedium(o, "extern.convert_any");
break;
default:
WASM_UNREACHABLE("invalid ref.is_*");
Expand Down
4 changes: 2 additions & 2 deletions src/passes/TypeGeneralizing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -850,10 +850,10 @@ struct TransferFn : OverriddenVisitor<TransferFn> {
case RefAsNonNull:
push(Type(type.getHeapType(), Nullable));
return;
case ExternInternalize:
case AnyConvertExtern:
push(Type(HeapType::ext, type.getNullability()));
return;
case ExternExternalize:
case ExternConvertAny:
push(Type(HeapType::any, type.getNullability()));
return;
}
Expand Down
2 changes: 1 addition & 1 deletion src/tools/wasm-ctor-eval.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -916,7 +916,7 @@ struct CtorEvalExternalInterface : EvallingModuleRunner::ExternalInterface {
if (original != value) {
// The original is externalized.
assert(original.type.getHeapType() == HeapType::ext);
ret = builder.makeRefAs(ExternExternalize, ret);
ret = builder.makeRefAs(ExternConvertAny, ret);
}
return ret;
}
Expand Down
4 changes: 2 additions & 2 deletions src/wasm-binary.h
Original file line number Diff line number Diff line change
Expand Up @@ -1120,8 +1120,8 @@ enum ASTNodes {
RefCastNull = 0x17,
BrOnCast = 0x18,
BrOnCastFail = 0x19,
ExternInternalize = 0x1a,
ExternExternalize = 0x1b,
AnyConvertExtern = 0x1a,
ExternConvertAny = 0x1b,
RefI31 = 0x1c,
I31GetS = 0x1d,
I31GetU = 0x1e,
Expand Down
2 changes: 1 addition & 1 deletion src/wasm-builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -1233,7 +1233,7 @@ class Builder {
return makeStringConst(wtf16.str());
}
if (type.isRef() && type.getHeapType() == HeapType::ext) {
return makeRefAs(ExternExternalize,
return makeRefAs(ExternConvertAny,
makeConstantExpression(value.internalize()));
}
TODO_SINGLE_COMPOUND(type);
Expand Down
6 changes: 3 additions & 3 deletions src/wasm-interpreter.h
Original file line number Diff line number Diff line change
Expand Up @@ -1858,9 +1858,9 @@ class ExpressionRunner : public OverriddenVisitor<SubType, Flow> {
trap("null ref");
}
return value;
case ExternInternalize:
case AnyConvertExtern:
return value.internalize();
case ExternExternalize:
case ExternConvertAny:
return value.externalize();
}
WASM_UNREACHABLE("unimplemented ref.as_*");
Expand Down Expand Up @@ -2436,7 +2436,7 @@ class ConstantExpressionRunner : public ExpressionRunner<SubType> {
}
Flow visitRefAs(RefAs* curr) {
// TODO: Remove this once interpretation is implemented.
if (curr->op == ExternInternalize || curr->op == ExternExternalize) {
if (curr->op == AnyConvertExtern || curr->op == ExternConvertAny) {
return Flow(NONCONSTANT_FLOW);
}
return ExpressionRunner<SubType>::visitRefAs(curr);
Expand Down
4 changes: 2 additions & 2 deletions src/wasm.h
Original file line number Diff line number Diff line change
Expand Up @@ -561,8 +561,8 @@ enum SIMDTernaryOp {

enum RefAsOp {
RefAsNonNull,
ExternInternalize,
ExternExternalize,
AnyConvertExtern,
ExternConvertAny,
};

enum BrOnOp {
Expand Down
12 changes: 6 additions & 6 deletions src/wasm/wasm-binary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4329,8 +4329,8 @@ BinaryConsts::ASTNodes WasmBinaryReader::readExpression(Expression*& curr) {
if (maybeVisitStringSliceWTF(curr, opcode)) {
break;
}
if (opcode == BinaryConsts::ExternInternalize ||
opcode == BinaryConsts::ExternExternalize) {
if (opcode == BinaryConsts::AnyConvertExtern ||
opcode == BinaryConsts::ExternConvertAny) {
visitRefAs((curr = allocator.alloc<RefAs>())->cast<RefAs>(), opcode);
break;
}
Expand Down Expand Up @@ -7730,11 +7730,11 @@ void WasmBinaryReader::visitRefAs(RefAs* curr, uint8_t code) {
case BinaryConsts::RefAsNonNull:
curr->op = RefAsNonNull;
break;
case BinaryConsts::ExternInternalize:
curr->op = ExternInternalize;
case BinaryConsts::AnyConvertExtern:
curr->op = AnyConvertExtern;
break;
case BinaryConsts::ExternExternalize:
curr->op = ExternExternalize;
case BinaryConsts::ExternConvertAny:
curr->op = ExternConvertAny;
break;
default:
WASM_UNREACHABLE("invalid code for ref.as_*");
Expand Down
8 changes: 4 additions & 4 deletions src/wasm/wasm-stack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2334,13 +2334,13 @@ void BinaryInstWriter::visitRefAs(RefAs* curr) {
case RefAsNonNull:
o << int8_t(BinaryConsts::RefAsNonNull);
break;
case ExternInternalize:
case AnyConvertExtern:
o << int8_t(BinaryConsts::GCPrefix)
<< U32LEB(BinaryConsts::ExternInternalize);
<< U32LEB(BinaryConsts::AnyConvertExtern);
break;
case ExternExternalize:
case ExternConvertAny:
o << int8_t(BinaryConsts::GCPrefix)
<< U32LEB(BinaryConsts::ExternExternalize);
<< U32LEB(BinaryConsts::ExternConvertAny);
break;
default:
WASM_UNREACHABLE("invalid ref.as_*");
Expand Down
12 changes: 6 additions & 6 deletions src/wasm/wasm-validator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2215,30 +2215,30 @@ void FunctionValidator::visitRefAs(RefAs* curr) {
default:
// TODO: validate all the other ref.as_*
break;
case ExternInternalize: {
case AnyConvertExtern: {
shouldBeTrue(getModule()->features.hasGC(),
curr,
"extern.internalize requries GC [--enable-gc]");
"any.convert_extern requries GC [--enable-gc]");
if (curr->type == Type::unreachable) {
return;
}
shouldBeSubType(curr->value->type,
Type(HeapType::ext, Nullable),
curr->value,
"extern.internalize value should be an externref");
"any.convert_extern value should be an externref");
break;
}
case ExternExternalize: {
case ExternConvertAny: {
shouldBeTrue(getModule()->features.hasGC(),
curr,
"extern.externalize requries GC [--enable-gc]");
"extern.convert_any requries GC [--enable-gc]");
if (curr->type == Type::unreachable) {
return;
}
shouldBeSubType(curr->value->type,
Type(HeapType::any, Nullable),
curr->value,
"extern.externalize value should be an anyref");
"extern.convert_any value should be an anyref");
break;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/wasm/wasm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1231,10 +1231,10 @@ void RefAs::finalize() {
case RefAsNonNull:
type = Type(value->type.getHeapType(), NonNullable);
break;
case ExternInternalize:
case AnyConvertExtern:
type = Type(HeapType::any, value->type.getNullability());
break;
case ExternExternalize:
case ExternConvertAny:
type = Type(HeapType::ext, value->type.getNullability());
break;
default:
Expand Down
4 changes: 2 additions & 2 deletions test/binaryen.js/expressions.js
Original file line number Diff line number Diff line change
Expand Up @@ -1451,7 +1451,7 @@ console.log("# RefAs");
assert(theRefAs.value === value);
assert(theRefAs.type !== binaryen.i32); // TODO: === (ref any)

theRefAs.op = op = binaryen.Operations.RefAsExternExternalize;
theRefAs.op = op = binaryen.Operations.RefAsExternConvertAny;
assert(theRefAs.op === op);
theRefAs.op = op = binaryen.Operations.RefAsNonNull;
theRefAs.value = value = module.local.get(2, binaryen.anyref);
Expand All @@ -1467,7 +1467,7 @@ console.log("# RefAs");
"(ref.as_non_null\n (local.get $2)\n)\n"
);

// TODO: extern.externalize and extern.internalize
// TODO: extern.convert_any and any.convert_extern

module.dispose();
})();
Expand Down
Loading

0 comments on commit 81f8f77

Please sign in to comment.