Reland "[dart2wasm] Use a separate Wasm struct for every Dart class"
This is a reland of commit 2cbb5eabca94ef7a2397bcd7844abad8ba9ca679
This removes the optimization that would reuse the Wasm struct of the
superclass if no new fields were added in a class. Such reuse
interferes with debugging.
Binaryen performs a similar optimization in its TypeMerging pass. It
seems to be doing a better job with the more precise type information
provided after this change. Benchmarks show a positive trend.
The change uncovered a number of latent bugs in the compiler, which
are also fixed in this CL:
- References to `StackTrace` objects must be typed with the
representation type of `StackTrace`, rather than its struct type.
- The dynamic call vtable entry and dynamic invocation forwarder
functions must use `_ListBase` rather than `_List` for their
parameters, since we sometimes pass growable lists to these
functions.
- Most covariant parameters must use the top type, since any object
can be passed into such parameters via class type parameter
covariance or tear-offs.
The change makes us fail a number of tests with very deep class
hierarchies due to the Wasm subtyping depth limit of 63. The
optimization did not absolve us from this limitation. It just made us
only count classes with newly added fields.
The limitation is not expected to cause any problems for real code.
The maximum subclassing depth that occurs in the Flute benchmark is
11. FWIW, Java has a limit of 60.
The limitation is mentioned along with other limitations in
https://github.com/dart-lang/sdk/issues/53703
Change-Id: I9f541049d2020934e98b2042a7d9852305f17ec1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/330341
Reviewed-by: Ömer Ağacan <omersa@google.com>
Commit-Queue: Ömer Ağacan <omersa@google.com>
diff --git a/pkg/dart2wasm/lib/async.dart b/pkg/dart2wasm/lib/async.dart
index 4264b4d..d4cb900 100644
--- a/pkg/dart2wasm/lib/async.dart
+++ b/pkg/dart2wasm/lib/async.dart
@@ -278,8 +278,8 @@
codeGen.b.catch_(codeGen.translator.exceptionTag);
- final stackTraceLocal =
- codeGen.addLocal(codeGen.translator.stackTraceInfo.nonNullableType);
+ final stackTraceLocal = codeGen
+ .addLocal(codeGen.translator.stackTraceInfo.repr.nonNullableType);
codeGen.b.local_set(stackTraceLocal);
final exceptionLocal =
@@ -573,8 +573,8 @@
asyncSuspendStateInfo.nonNullableType, // _AsyncSuspendState
translator.topInfo.nullableType, // Object?, await value
translator.topInfo.nullableType, // Object?, error value
- translator
- .stackTraceInfo.nullableType // StackTrace?, error stack trace
+ translator.stackTraceInfo.repr
+ .nullableType // StackTrace?, error stack trace
], [
// Inner function does not return a value, but it's Dart type is
// `void Function(...)` and all Dart functions return a value, so we
@@ -647,7 +647,7 @@
b.local_get(asyncStateLocal);
b.ref_null(translator.topInfo.struct); // await value
b.ref_null(translator.topInfo.struct); // error value
- b.ref_null(translator.stackTraceInfo.struct); // stack trace
+ b.ref_null(translator.stackTraceInfo.repr.struct); // stack trace
b.call(resumeFun);
b.drop(); // drop null
@@ -783,7 +783,8 @@
b.catch_(translator.exceptionTag);
- final stackTraceLocal = addLocal(translator.stackTraceInfo.nonNullableType);
+ final stackTraceLocal =
+ addLocal(translator.stackTraceInfo.repr.nonNullableType);
b.local_set(stackTraceLocal);
final exceptionLocal = addLocal(translator.topInfo.nonNullableType);
@@ -1291,7 +1292,8 @@
wrap(node.expression, translator.topInfo.nonNullableType);
b.local_set(exceptionLocal);
- final stackTraceLocal = addLocal(translator.stackTraceInfo.nonNullableType);
+ final stackTraceLocal =
+ addLocal(translator.stackTraceInfo.repr.nonNullableType);
call(translator.stackTraceCurrent.reference);
b.local_set(stackTraceLocal);
diff --git a/pkg/dart2wasm/lib/class_info.dart b/pkg/dart2wasm/lib/class_info.dart
index e56af9b..85afb84 100644
--- a/pkg/dart2wasm/lib/class_info.dart
+++ b/pkg/dart2wasm/lib/class_info.dart
@@ -366,19 +366,8 @@
}
}
- // A class can reuse the Wasm struct of the superclass if it doesn't
- // declare any Wasm fields of its own. This is the case when three
- // conditions are met:
- // 1. All type parameters can reuse a type parameter field of the
- // superclass.
- // 2. The class declares no Dart fields of its own.
- // 3. The class is not a special class that contains hidden fields.
- bool canReuseSuperStruct =
- typeParameterMatch.length == cls.typeParameters.length &&
- cls.fields.where((f) => f.isInstanceMember).isEmpty;
- w.StructType struct = canReuseSuperStruct
- ? superInfo.struct
- : m.types.defineStruct(cls.name, superType: superInfo.struct);
+ w.StructType struct =
+ m.types.defineStruct(cls.name, superType: superInfo.struct);
info = ClassInfo(
cls, _nextClassId++, superInfo.depth + 1, struct, superInfo,
typeParameterMatch: typeParameterMatch);
@@ -447,7 +436,7 @@
// Top - add class id field
info._addField(
w.FieldType(w.NumType.i32, mutable: false), FieldIndex.classId);
- } else if (info.struct != superInfo.struct) {
+ } else {
// Copy fields from superclass
for (w.FieldType fieldType in superInfo.struct.fields) {
info._addField(fieldType);
@@ -476,12 +465,6 @@
info._addField(w.FieldType(wasmType, mutable: !field.isFinal));
}
}
- } else {
- for (TypeParameter parameter in info.cls!.typeParameters) {
- // Reuse supertype type variable
- translator.typeParameterIndex[parameter] =
- translator.typeParameterIndex[info.typeParameterMatch[parameter]]!;
- }
}
}
diff --git a/pkg/dart2wasm/lib/code_generator.dart b/pkg/dart2wasm/lib/code_generator.dart
index 3ae74b7..75953b9 100644
--- a/pkg/dart2wasm/lib/code_generator.dart
+++ b/pkg/dart2wasm/lib/code_generator.dart
@@ -1150,7 +1150,7 @@
// stack after each type test. Also, store the stack trace in a local.
w.Local thrownException = addLocal(translator.topInfo.nonNullableType);
w.Local thrownStackTrace =
- addLocal(translator.stackTraceInfo.nonNullableType);
+ addLocal(translator.stackTraceInfo.repr.nonNullableType);
void emitCatchBlock(Catch catch_, bool emitGuard) {
// For each catch node:
@@ -1864,7 +1864,7 @@
translator.types.makeType(this, typeArguments[elementIdx]);
}, isGrowable: true);
final typeArgsLocal = function.addLocal(
- translator.classInfo[translator.fixedLengthListClass]!.nonNullableType);
+ translator.classInfo[translator.growableListClass]!.nonNullableType);
b.local_set(typeArgsLocal);
// Evaluate positional arguments
@@ -3707,9 +3707,10 @@
compare = () => codeGen.b.i64_eq();
} else if (check<StringLiteral, StringConstant>()) {
// String switch
- nonNullableType =
- translator.classInfo[translator.stringBaseClass]!.nonNullableType;
- nullableType = nonNullableType.withNullability(true);
+ nonNullableType = translator
+ .classInfo[translator.coreTypes.stringClass]!.repr.nonNullableType;
+ nullableType = translator
+ .classInfo[translator.coreTypes.stringClass]!.repr.nullableType;
compare = () => codeGen.call(translator.stringEquals.reference);
} else {
// Object switch
diff --git a/pkg/dart2wasm/lib/dispatch_table.dart b/pkg/dart2wasm/lib/dispatch_table.dart
index ba84724..c43fa4a 100644
--- a/pkg/dart2wasm/lib/dispatch_table.dart
+++ b/pkg/dart2wasm/lib/dispatch_table.dart
@@ -41,6 +41,9 @@
/// Is this an implicit or explicit setter?
final bool isSetter;
+ /// Does this method have any tear-off uses?
+ bool hasTearOffUses = false;
+
/// Maps class IDs to the selector's member in the class. The member can be
/// abstract.
final Map<int, Reference> targets = {};
@@ -113,28 +116,39 @@
named = const {};
returns = [function.computeFunctionType(Nullability.nonNullable)];
} else {
+ DartType typeForParam(VariableDeclaration param, int index) {
+ if (param.isCovariantByClass) {
+ // The type argument of a static type is not required to conform
+ // to the bounds of the type variable. Thus, any object can be
+ // passed to a parameter that is covariant by class.
+ return translator.coreTypes.objectNullableRawType;
+ }
+ if (param.isCovariantByDeclaration) {
+ if (hasTearOffUses) {
+ // The type of a covariant parameter in the runtime function
+ // type of a tear-off is always `Object?`. Thus, if the method
+ // has any tear-off uses, any object could be passed into the
+ // parameter.
+ return translator.coreTypes.objectNullableRawType;
+ }
+ if (!translator.options.omitTypeChecks) {
+ // A runtime type check of the parameter will be generated.
+ // The value therefore must be boxed.
+ ensureBoxed[index] = true;
+ }
+ }
+ return param.type;
+ }
+
positional = [
- for (VariableDeclaration param in function.positionalParameters)
- param.type
+ for (int i = 0; i < function.positionalParameters.length; i++)
+ typeForParam(function.positionalParameters[i], 1 + i)
];
named = {
for (VariableDeclaration param in function.namedParameters)
- param.name!: param.type
+ param.name!: typeForParam(param, 1 + nameIndex[param.name!]!)
};
returns = target.isSetter ? const [] : [function.returnType];
-
- // Box parameters that need covariance checks
- if (!translator.options.omitTypeChecks) {
- for (int i = 0; i < function.positionalParameters.length; i += 1) {
- final param = function.positionalParameters[i];
- ensureBoxed[1 + i] |=
- param.isCovariantByClass || param.isCovariantByDeclaration;
- }
- for (VariableDeclaration param in function.namedParameters) {
- ensureBoxed[1 + nameIndex[param.name!]!] |=
- param.isCovariantByClass || param.isCovariantByDeclaration;
- }
- }
}
}
assert(returns.length <= outputSets.length);
@@ -287,6 +301,7 @@
_selectorMetadata[selectorId].callCount, paramInfo,
isSetter: isSetter));
assert(selector.isSetter == isSetter);
+ selector.hasTearOffUses |= metadata.hasTearOffUses;
selector.paramInfo.merge(paramInfo);
if (calledDynamically) {
if (isGetter) {
diff --git a/pkg/dart2wasm/lib/sync_star.dart b/pkg/dart2wasm/lib/sync_star.dart
index 95cc5f3..70190e2 100644
--- a/pkg/dart2wasm/lib/sync_star.dart
+++ b/pkg/dart2wasm/lib/sync_star.dart
@@ -241,7 +241,7 @@
m.types.defineFunction([
suspendStateInfo.nonNullableType,
translator.topInfo.nullableType,
- translator.stackTraceInfo.nullableType
+ translator.stackTraceInfo.repr.nullableType
], const [
w.NumType.i32
]),
diff --git a/pkg/dart2wasm/lib/translator.dart b/pkg/dart2wasm/lib/translator.dart
index ab1d15c..58c0f25 100644
--- a/pkg/dart2wasm/lib/translator.dart
+++ b/pkg/dart2wasm/lib/translator.dart
@@ -173,13 +173,13 @@
w.RefType.def(closureLayouter.closureBaseStruct, nullable: false),
// Type arguments
- classInfo[fixedLengthListClass]!.nonNullableType,
+ classInfo[listBaseClass]!.nonNullableType,
// Positional arguments
- classInfo[fixedLengthListClass]!.nonNullableType,
+ classInfo[listBaseClass]!.nonNullableType,
// Named arguments, represented as array of symbol and object pairs
- classInfo[fixedLengthListClass]!.nonNullableType,
+ classInfo[listBaseClass]!.nonNullableType,
], [
topInfo.nullableType
]);
@@ -191,13 +191,13 @@
topInfo.nonNullableType,
// Type arguments
- classInfo[fixedLengthListClass]!.nonNullableType,
+ classInfo[listBaseClass]!.nonNullableType,
// Positional arguments
- classInfo[fixedLengthListClass]!.nonNullableType,
+ classInfo[listBaseClass]!.nonNullableType,
// Named arguments, represented as array of symbol and object pairs
- classInfo[fixedLengthListClass]!.nonNullableType,
+ classInfo[listBaseClass]!.nonNullableType,
], [
topInfo.nullableType
]);
@@ -465,7 +465,8 @@
/// exception tag is used to throw and catch all Dart exceptions.
w.Tag createExceptionTag() {
w.FunctionType tagType = m.types.defineFunction(
- [topInfo.nonNullableType, stackTraceInfo.nonNullableType], const []);
+ [topInfo.nonNullableType, stackTraceInfo.repr.nonNullableType],
+ const []);
w.Tag tag = m.tags.define(tagType);
return tag;
}
