Skip to content

Commit

Permalink
[dart2wasm] Remove support for not using non-nullable locals.
Browse files Browse the repository at this point in the history
With the decision to support non-nullable locals in WasmGC as per
WebAssembly/function-references#44 the
support in dart2wasm for forcing all locals to be nullable is no
longer needed.

This CL removes that support and cleans up some related nullability
issues. Specifically:

- Remove the `--local-nullability` and `--parameter-nullability`
  commandline options. These are now always enabled.
- Clean out special cases around forced nullable locals throughout the
  compiler.
- Make `thisLocal` and `preciseThisLocal` always non-nullable.
- Make `returnValueLocal` (for storing the return value of `return`
  statements inside `try` blocks with `finally`) always defaultable,
  since its initialization flow crosses control constructs.
- Make type argument parameters non-nullable.
- Make non-nullable `FutureOr` translate to a non-nullable Wasm type.
- Implement the "initialized until end of block" validation scheme
  in the Wasm instruction validator.
- Run tests with the `--experimental-wasm-nn-locals` option. This
  is likely going away soon, but for now we need it.

Change-Id: I05873dd70510af5944d86d37cb5765c7bdef73a9
Cq-Include-Trybots: luci.dart.try:dart2wasm-linux-x64-d8-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/252600
Commit-Queue: Aske Simon Christensen <askesc@google.com>
Reviewed-by: Joshua Litt <joshualitt@google.com>
Reviewed-by: William Hesse <whesse@google.com>
  • Loading branch information
askeksa authored and Commit Bot committed Jul 27, 2022
1 parent e75be08 commit 9814b56
Show file tree
Hide file tree
Showing 13 changed files with 82 additions and 92 deletions.
6 changes: 0 additions & 6 deletions pkg/dart2wasm/bin/dart2wasm.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,10 @@ final List<Option> options = [
Flag(
"lazy-constants", (o, value) => o.translatorOptions.lazyConstants = value,
defaultsTo: _d.translatorOptions.lazyConstants),
Flag("local-nullability",
(o, value) => o.translatorOptions.localNullability = value,
defaultsTo: _d.translatorOptions.localNullability),
Flag("name-section", (o, value) => o.translatorOptions.nameSection = value,
defaultsTo: _d.translatorOptions.nameSection),
Flag("nominal-types", (o, value) => o.translatorOptions.nominalTypes = value,
defaultsTo: _d.translatorOptions.nominalTypes),
Flag("parameter-nullability",
(o, value) => o.translatorOptions.parameterNullability = value,
defaultsTo: _d.translatorOptions.parameterNullability),
Flag("polymorphic-specialization",
(o, value) => o.translatorOptions.polymorphicSpecialization = value,
defaultsTo: _d.translatorOptions.polymorphicSpecialization),
Expand Down
2 changes: 1 addition & 1 deletion pkg/dart2wasm/bin/run_wasm.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
//
// Run as follows:
//
// $> d8 --experimental-wasm-gc --wasm-gc-js-interop run_wasm.js -- <dart_module>.wasm [<ffi_module>.wasm]
// $> d8 --experimental-wasm-gc --experimental-wasm-nn-locals --wasm-gc-js-interop run_wasm.js -- <dart_module>.wasm [<ffi_module>.wasm]
//
// If an FFI module is specified, it will be instantiated first, and its
// exports will be supplied as imports to the Dart module under the 'ffi'
Expand Down
4 changes: 1 addition & 3 deletions pkg/dart2wasm/dart2wasm.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,8 @@ where *options* include:
| `--`[`no-`]`import-shared-memory` | no | Import a shared memory buffer. If this is on, `--shared-memory-max-pages` must also be specified.
| `--`[`no-`]`inlining` | no | Inline small functions.
| `--`[`no-`]`lazy-constants` | no | Instantiate constants lazily.
| `--`[`no-`]`local-nullability` | no | Use non-nullable types for non-nullable locals and temporaries.
| `--`[`no-`]`name-section` | yes | Emit Name Section with function names.
| `--`[`no-`]`nominal-types` | yes | Emit nominal types.
| `--`[`no-`]`parameter-nullability` | yes | Use non-nullable types for non-nullable parameters and return values.
| `--`[`no-`]`polymorphic-specialization` | no | Do virtual calls by switching on the class ID instead of using `call_indirect`.
| `--`[`no-`]`print-kernel` | no | Print IR for each function before compiling it.
| `--`[`no-`]`print-wasm` | no | Print Wasm instructions of each compiled function.
Expand All @@ -30,7 +28,7 @@ where *options* include:

The resulting `.wasm` file can be run with:

`d8 --experimental-wasm-gc --wasm-gc-js-interop pkg/dart2wasm/bin/run_wasm.js -- `*outfile*`.wasm`
`d8 --experimental-wasm-gc --experimental-wasm-nn-locals --wasm-gc-js-interop pkg/dart2wasm/bin/run_wasm.js -- `*outfile*`.wasm`

## Imports and exports

Expand Down
69 changes: 23 additions & 46 deletions pkg/dart2wasm/lib/code_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>
w.ValueType translateType(DartType type) => translator.translateType(type);

w.Local addLocal(w.ValueType type) {
return function.addLocal(translator.typeForLocal(type));
return function.addLocal(type);
}

DartType dartTypeOf(Expression exp) {
Expand Down Expand Up @@ -205,7 +205,7 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>

void getThis() {
w.Local thisLocal = paramLocals[0];
w.RefType structType = w.RefType.def(struct, nullable: true);
w.RefType structType = w.RefType.def(struct, nullable: false);
b.local_get(thisLocal);
translator.convertType(function, thisLocal.type, structType);
}
Expand Down Expand Up @@ -338,16 +338,16 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>
if (context.parent == null) break;

b.struct_get(context.struct, context.parentFieldIndex);
if (options.localNullability) {
b.ref_as_non_null();
}
b.ref_as_non_null();
context = context.parent!;
}
if (context.containsThis) {
thisLocal = addLocal(
context.struct.fields[context.thisFieldIndex].type.unpacked);
thisLocal = addLocal(context
.struct.fields[context.thisFieldIndex].type.unpacked
.withNullability(false));
preciseThisLocal = thisLocal;
b.struct_get(context.struct, context.thisFieldIndex);
b.ref_as_non_null();
b.local_set(thisLocal!);
}
}
Expand Down Expand Up @@ -487,9 +487,6 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>
void visitRedirectingInitializer(RedirectingInitializer node) {
Class cls = (node.parent as Constructor).enclosingClass;
b.local_get(thisLocal!);
if (options.parameterNullability && thisLocal!.type.nullable) {
b.ref_as_non_null();
}
for (TypeParameter typeParam in cls.typeParameters) {
types.makeType(
this, TypeParameterType(typeParam, Nullability.nonNullable));
Expand All @@ -507,9 +504,6 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>
return;
}
b.local_get(thisLocal!);
if (options.parameterNullability && thisLocal!.type.nullable) {
b.ref_as_non_null();
}
for (DartType typeArg in supertype!.typeArguments) {
types.makeType(this, typeArg);
}
Expand Down Expand Up @@ -882,7 +876,10 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>
finalizer.mustHandleReturn = true;
}
if (returnType != voidMarker) {
returnValueLocal ??= addLocal(returnType);
// Since the flow of the return value through the returnValueLocal
// crosses control-flow constructs, the local needs to always have a
// defaultable type in order for the Wasm code to validate.
returnValueLocal ??= addLocal(returnType.withNullability(true));
b.local_set(returnValueLocal!);
}
b.br(finalizers.last.label);
Expand Down Expand Up @@ -1001,8 +998,6 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>
} else {
wrap(exp, nonNullableType);
b.local_get(switchValueNonNullableLocal);
translator.convertType(
function, switchValueNonNullableLocal.type, nonNullableType);
compare();
b.br_if(switchLabels[c]!);
}
Expand Down Expand Up @@ -1095,15 +1090,17 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>
}

w.ValueType visitThis(w.ValueType expectedType) {
w.ValueType thisType = thisLocal!.type.withNullability(false);
w.ValueType preciseThisType = preciseThisLocal!.type.withNullability(false);
w.ValueType thisType = thisLocal!.type;
w.ValueType preciseThisType = preciseThisLocal!.type;
assert(!thisType.nullable);
assert(!preciseThisType.nullable);
if (!thisType.isSubtypeOf(expectedType) &&
preciseThisType.isSubtypeOf(expectedType)) {
b.local_get(preciseThisLocal!);
return preciseThisLocal!.type;
return preciseThisType;
} else {
b.local_get(thisLocal!);
return thisLocal!.type;
return thisType;
}
}

Expand All @@ -1122,9 +1119,6 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>
b.local_get(temp);
b.i32_const(info.classId);
b.struct_set(info.struct, FieldIndex.classId);
if (options.parameterNullability && temp.type.nullable) {
b.ref_as_non_null();
}
_visitArguments(node.arguments, node.targetReference, 1);
call(node.targetReference);
if (expectedType != voidMarker) {
Expand Down Expand Up @@ -1158,8 +1152,7 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>
_lookupSuperTarget(node.interfaceTarget, setter: false).reference;
w.BaseFunction targetFunction = translator.functions.getFunction(target);
w.ValueType receiverType = targetFunction.type.inputs.first;
w.ValueType thisType = visitThis(receiverType);
translator.convertType(function, thisType, receiverType);
visitThis(receiverType);
_visitArguments(node.arguments, target, 1);
return call(target);
}
Expand Down Expand Up @@ -1261,14 +1254,12 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>
b.local_get(leftLocal);
if (leftNullable) {
b.br_on_null(operandNull!);
} else if (leftLocal.type.nullable) {
b.ref_as_non_null();
}
}

void right([_]) {
b.local_get(rightLocal);
if (rightLocal.type.nullable) {
if (rightNullable) {
b.ref_as_non_null();
}
}
Expand Down Expand Up @@ -1339,10 +1330,8 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>

// Receiver is already on stack.
w.Local receiverVar = addLocal(selector.signature.inputs.first);
assert(!receiverVar.type.nullable);
b.local_tee(receiverVar);
if (options.parameterNullability && receiverVar.type.nullable) {
b.ref_as_non_null();
}
pushArguments(selector.signature);

if (options.polymorphicSpecialization) {
Expand Down Expand Up @@ -1610,10 +1599,8 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>
// Evaluate receiver
wrap(node.receiver, selector.signature.inputs.first);
w.Local receiverVar = addLocal(selector.signature.inputs.first);
assert(!receiverVar.type.nullable);
b.local_tee(receiverVar);
if (options.parameterNullability && receiverVar.type.nullable) {
b.ref_as_non_null();
}

// Dispatch table call
b.comment("Dynamic get of '${selector.name}'");
Expand Down Expand Up @@ -1714,7 +1701,6 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>
if (preserved) {
temp = addLocal(paramType);
b.local_tee(temp);
translator.convertType(function, temp.type, paramType);
}
call(target.reference);
}
Expand Down Expand Up @@ -1779,9 +1765,7 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>
Context? context = closures.contexts[functionNode]?.parent;
if (context != null) {
b.local_get(context.currentLocal);
if (context.currentLocal.type.nullable) {
b.ref_as_non_null();
}
assert(!context.currentLocal.type.nullable);
} else {
b.global_get(translator.globals.dummyGlobal); // Dummy context
}
Expand Down Expand Up @@ -1908,7 +1892,6 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>
signature.inputs[signatureOffset + paramInfo.nameIndex[name]!];
if (namedLocal != null) {
b.local_get(namedLocal);
translator.convertType(function, namedLocal.type, type);
} else {
translator.constants
.instantiateConstant(function, b, paramInfo.named[name]!, type);
Expand Down Expand Up @@ -2029,9 +2012,6 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>
b.array_set(arrayType);
}
b.local_get(arrayLocal);
if (arrayLocal.type.nullable) {
b.ref_as_non_null();
}
}
} else {
for (int i = 0; i < length; i++) {
Expand Down Expand Up @@ -2072,7 +2052,6 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>
b.local_set(mapLocal);
for (MapLiteralEntry entry in node.entries) {
b.local_get(mapLocal);
translator.convertType(function, mapLocal.type, putReceiverType);
wrap(entry.key, putKeyType);
wrap(entry.value, putValueType);
b.call(mapPut);
Expand Down Expand Up @@ -2100,7 +2079,6 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>
b.local_set(setLocal);
for (Expression element in node.expressions) {
b.local_get(setLocal);
translator.convertType(function, setLocal.type, addReceiverType);
wrap(element, addKeyType);
b.call(setAdd);
b.drop();
Expand Down Expand Up @@ -2161,8 +2139,7 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>
Class cls = parameter.parent as Class;
ClassInfo info = translator.classInfo[cls]!;
int fieldIndex = translator.typeParameterIndex[parameter]!;
w.ValueType thisType = visitThis(info.nonNullableType);
translator.convertType(function, thisType, info.nonNullableType);
visitThis(info.nonNullableType);
b.struct_get(info.struct, fieldIndex);
resultType = info.struct.fields[fieldIndex].type.unpacked;
}
Expand Down
13 changes: 4 additions & 9 deletions pkg/dart2wasm/lib/constants.dart
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,8 @@ class Constants {

w.Local offset = function.locals[0];
w.Local length = function.locals[1];
w.Local array = function.addLocal(
translator.typeForLocal(w.RefType.def(arrayType, nullable: false)));
w.Local array =
function.addLocal(w.RefType.def(arrayType, nullable: false));
w.Local index = function.addLocal(w.NumType.i32);

w.Instructions b = function.body;
Expand Down Expand Up @@ -422,12 +422,11 @@ class ConstantCreator extends ConstantVisitor<ConstantInfo?> {
w.FunctionType ftype = translator.functionType(const [], [type]);
w.DefinedFunction function = m.addFunction(ftype, "$constant");
generator(function, function.body);
w.Local temp = function.addLocal(translator.typeForLocal(type));
w.Local temp = function.addLocal(type);
w.Instructions b2 = function.body;
b2.local_tee(temp);
b2.global_set(global);
b2.local_get(temp);
translator.convertType(function, temp.type, type);
b2.end();

return ConstantInfo(constant, global, function);
Expand Down Expand Up @@ -579,8 +578,7 @@ class ConstantCreator extends ConstantVisitor<ConstantInfo?> {
b.i64_const(length);
if (lazyConstants) {
// Allocate array and set each entry to the corresponding sub-constant.
w.Local arrayLocal = function!.addLocal(
refType.withNullability(!translator.options.localNullability));
w.Local arrayLocal = function!.addLocal(refType.withNullability(false));
b.i32_const(length);
translator.array_new_default(b, arrayType);
b.local_set(arrayLocal);
Expand All @@ -592,9 +590,6 @@ class ConstantCreator extends ConstantVisitor<ConstantInfo?> {
b.array_set(arrayType);
}
b.local_get(arrayLocal);
if (arrayLocal.type.nullable) {
b.ref_as_non_null();
}
} else {
// Push all sub-constants on the stack and initialize array from them.
for (int i = 0; i < length; i++) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/dart2wasm/lib/dispatch_table.dart
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ class SelectorInfo {
});

List<w.ValueType> typeParameters = List.filled(paramInfo.typeParamCount,
translator.classInfo[translator.typeClass]!.nullableType);
translator.classInfo[translator.typeClass]!.nonNullableType);
List<w.ValueType> inputs = List.generate(
inputSets.length,
(i) => translator.typeForInfo(
Expand Down
2 changes: 1 addition & 1 deletion pkg/dart2wasm/lib/functions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ class FunctionCollector extends MemberVisitor1<w.FunctionType, Reference> {
}

List<w.ValueType> typeParameters = List.filled(typeParamCount,
translator.classInfo[translator.typeClass]!.nullableType);
translator.classInfo[translator.typeClass]!.nonNullableType);

// The JS embedder will not accept Wasm struct types as parameter or return
// types for functions called from JS. We need to use eqref instead.
Expand Down
3 changes: 1 addition & 2 deletions pkg/dart2wasm/lib/intrinsics.dart
Original file line number Diff line number Diff line change
Expand Up @@ -811,7 +811,7 @@ class Intrinsifier {
codeGen.wrap(
node.arguments.positional.single, receiverInfo.nonNullableType);
w.Local receiverLocal =
codeGen.function.addLocal(receiverInfo.nullableType);
codeGen.function.addLocal(receiverInfo.nonNullableType);
b.local_tee(receiverLocal);
// We ignore the type argument and just update the classID of the
// receiver.
Expand All @@ -823,7 +823,6 @@ class Intrinsifier {
b.i32_const(fixedLengthListInfo.classId);
b.struct_set(topInfo.struct, FieldIndex.classId);
b.local_get(receiverLocal);
b.ref_as_non_null();
return fixedLengthListInfo.nonNullableType;
}
}
Expand Down
Loading

0 comments on commit 9814b56

Please sign in to comment.