[dart2wasm] Use a separate Wasm struct for every Dart class
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: Ifaa23d8278989e2549c41696d8913e55ed1e80aa Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/324460 Commit-Queue: Aske Simon Christensen <askesc@google.com> Reviewed-by: Ömer Ağacan <omersa@google.com>
This commit is contained in:
committed by
Commit Queue
parent
eaa308fbef
commit
2cbb5eabca
@@ -278,8 +278,8 @@ class _ExceptionHandlerStack {
|
||||
|
||||
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 @@ class AsyncCodeGenerator extends CodeGenerator {
|
||||
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 @@ class AsyncCodeGenerator extends CodeGenerator {
|
||||
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 @@ class AsyncCodeGenerator extends CodeGenerator {
|
||||
|
||||
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 @@ class AsyncCodeGenerator extends CodeGenerator {
|
||||
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);
|
||||
|
||||
|
||||
@@ -366,19 +366,8 @@ class ClassInfoCollector {
|
||||
}
|
||||
}
|
||||
|
||||
// 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 @@ class ClassInfoCollector {
|
||||
// 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 @@ class ClassInfoCollector {
|
||||
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]]!;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1150,7 +1150,7 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>
|
||||
// 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 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>
|
||||
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
|
||||
|
||||
@@ -41,6 +41,9 @@ class SelectorInfo {
|
||||
/// 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 @@ class SelectorInfo {
|
||||
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 @@ class DispatchTable {
|
||||
_selectorMetadata[selectorId].callCount, paramInfo,
|
||||
isSetter: isSetter));
|
||||
assert(selector.isSetter == isSetter);
|
||||
selector.hasTearOffUses |= metadata.hasTearOffUses;
|
||||
selector.paramInfo.merge(paramInfo);
|
||||
if (calledDynamically) {
|
||||
if (isGetter) {
|
||||
|
||||
@@ -241,7 +241,7 @@ class SyncStarCodeGenerator extends CodeGenerator {
|
||||
m.types.defineFunction([
|
||||
suspendStateInfo.nonNullableType,
|
||||
translator.topInfo.nullableType,
|
||||
translator.stackTraceInfo.nullableType
|
||||
translator.stackTraceInfo.repr.nullableType
|
||||
], const [
|
||||
w.NumType.i32
|
||||
]),
|
||||
|
||||
@@ -173,13 +173,13 @@ class Translator with KernelNodes {
|
||||
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 @@ class Translator with KernelNodes {
|
||||
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 @@ class Translator with KernelNodes {
|
||||
/// 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;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user