Skip to content

Commit

Permalink
[dart2wasm] Disentagle dispatch table calls from static calls in dart…
Browse files Browse the repository at this point in the history
…2wasm

Not all instance members are necessarily called via dispatch table
calls.

If we know an instance member isn't a target of any dispatch table call
we can choose the representation for that member on its own (instead of
using the selector representation - which is a upper-bounding of all
implementations of that selector).

This CL requires all call sites to obtain target signature & parameter
information via one of

  * `Translator.{signature,paramInfo}ForDirectCall`
  * `Translator.{signature,paramInfo}ForDispatchTableCall`

=> Those methods handle the case of calling a method

   a) via dispatch table call
   b) via direct call where
     b.1) target can also be called via dispatch table
     b.2) target cannot be called via dispatch table

In a follow up CL we're going to reland [0] which will shrink the set of
instance methods that are callable via dispatch table calls (e.g.
members that are only called via `super.*()` can use their own
representation selection instead of clustering it with methods of same
name that can be called via dispatch table).

The regression test added in this CL is a test that will fail with [0]
without this CL.

[0] https://dart-review.googlesource.com/c/sdk/+/374320

Change-Id: I5184cf6e712f33a894fd1463a5903128f87be92f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/375280
Commit-Queue: Martin Kustermann <[email protected]>
Reviewed-by: Slava Egorov <[email protected]>
  • Loading branch information
mkustermann committed Jul 11, 2024
1 parent 0313dbd commit 0889fdd
Show file tree
Hide file tree
Showing 8 changed files with 215 additions and 103 deletions.
4 changes: 2 additions & 2 deletions pkg/dart2wasm/lib/async.dart
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,8 @@ class AsyncCodeGenerator extends StateMachineCodeGenerator {
// (3) Return the completer's future.

b.local_get(asyncStateLocal);
final completerFutureGetterType = translator.functions
.getFunctionType(translator.completerFuture.getterReference);
final completerFutureGetterType = translator
.signatureForDirectCall(translator.completerFuture.getterReference);
b.struct_get(
asyncSuspendStateInfo.struct, FieldIndex.asyncSuspendStateCompleter);
translator.convertType(
Expand Down
122 changes: 71 additions & 51 deletions pkg/dart2wasm/lib/code_generator.dart

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions pkg/dart2wasm/lib/dispatch_table.dart
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ class SelectorInfo {
/// Maps class IDs to the selector's member in the class. The member can be
/// abstract.
final Map<int, Reference> targets = {};
late final Set<Reference> targetSet = targets.values.toSet();

/// Wasm function type for the selector.
///
Expand Down
2 changes: 1 addition & 1 deletion pkg/dart2wasm/lib/dynamic_forwarders.dart
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ class Forwarder {
continue;
}

final targetMemberParamInfo = translator.paramInfoFor(target);
final targetMemberParamInfo = translator.paramInfoForDirectCall(target);

b.local_get(receiverLocal);
b.struct_get(translator.topInfo.struct, FieldIndex.classId);
Expand Down
90 changes: 58 additions & 32 deletions pkg/dart2wasm/lib/functions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -134,53 +134,62 @@ class FunctionCollector {
w.BaseFunction getFunction(Reference target) {
return _functions.putIfAbsent(target, () {
_worklist.add(target);
return _getFunctionTypeAndName(target, m.functions.define);
return m.functions.define(
translator.signatureForDirectCall(target), _getFunctionName(target));
});
}

w.FunctionType getFunctionType(Reference target) {
return _getFunctionTypeAndName(target, (ftype, name) => ftype);
return _getFunctionType(target);
}

/// Pass the Wasm type and name of the function for [target] to [action].
///
/// Name should be used for the Wasm names section entry for the function so
/// that the error stack traces will have names expected by the Dart spec.
T _getFunctionTypeAndName<T>(
Reference target, T Function(w.FunctionType, String) action) {
w.FunctionType _getFunctionType(Reference target) {
final Member member = target.asMember;
if (target.isTypeCheckerReference) {
Member member = target.asMember;
if (member is Field || (member is Procedure && member.isSetter)) {
return action(translator.dynamicSetForwarderFunctionType,
'${target.asMember} setter type checker');
return translator.dynamicSetForwarderFunctionType;
} else {
return action(translator.dynamicInvocationForwarderFunctionType,
'${target.asMember} invocation type checker');
return translator.dynamicInvocationForwarderFunctionType;
}
}

if (target.isTearOffReference) {
return action(
translator.dispatchTable.selectorForTarget(target).signature,
"${target.asMember} tear-off");
assert(!translator.dispatchTable
.selectorForTarget(target)
.targetSet
.contains(target));
return translator.signatureForDirectCall(target);
}

Member member = target.asMember;
final ftype = member.accept1(_FunctionTypeGenerator(translator), target);
return member.accept1(_FunctionTypeGenerator(translator), target);
}

String _getFunctionName(Reference target) {
if (target.isTearOffReference) {
return "${target.asMember} tear-off";
}

final Member member = target.asMember;
String memberName = member.toString();
if (memberName.endsWith('.')) {
memberName = memberName.substring(0, memberName.length - 1);
}
if (target.isTypeCheckerReference) {
if (member is Field || (member is Procedure && member.isSetter)) {
return '$memberName setter type checker';
} else {
return '$memberName invocation type checker';
}
}

if (target.isInitializerReference) {
return action(ftype, 'new $memberName (initializer)');
return 'new $memberName (initializer)';
} else if (target.isConstructorBodyReference) {
return action(ftype, 'new $memberName (constructor body)');
return 'new $memberName (constructor body)';
} else if (member is Procedure && member.isFactory) {
return action(ftype, 'new $memberName');
return 'new $memberName';
} else {
return action(ftype, memberName);
return memberName;
}
}

Expand Down Expand Up @@ -229,15 +238,33 @@ class _FunctionTypeGenerator extends MemberVisitor1<w.FunctionType, Reference> {
String kind = target == node.setterReference ? "setter" : "getter";
throw "No implicit $kind function for static field: $node";
}
return translator.dispatchTable.selectorForTarget(target).signature;
assert(!translator.dispatchTable
.selectorForTarget(target)
.targetSet
.contains(target));

final receiverType = target.asMember.enclosingClass!
.getThisType(translator.coreTypes, Nullability.nonNullable);
return _makeFunctionType(
translator, target, translator.translateType(receiverType));
}

@override
w.FunctionType visitProcedure(Procedure node, Reference target) {
assert(!node.isAbstract);
return node.isInstanceMember
? translator.dispatchTable.selectorForTarget(node.reference).signature
: _makeFunctionType(translator, target, null);
if (!node.isInstanceMember) {
return _makeFunctionType(translator, target, null);
}

assert(!translator.dispatchTable
.selectorForTarget(target)
.targetSet
.contains(target));

final receiverType = target.asMember.enclosingClass!
.getThisType(translator.coreTypes, Nullability.nonNullable);
return _makeFunctionType(
translator, target, translator.translateType(receiverType));
}

@override
Expand Down Expand Up @@ -291,8 +318,8 @@ class _FunctionTypeGenerator extends MemberVisitor1<w.FunctionType, Reference> {

if (supersupertype != null) {
ClassInfo superInfo = info.superInfo!;
w.FunctionType superInitializer = translator.functions
.getFunctionType(initializer.target.initializerReference);
w.FunctionType superInitializer = translator
.signatureForDirectCall(initializer.target.initializerReference);

final int numSuperclassFields = superInfo.getClassFieldTypes().length;
final int numSuperContextAndConstructorArgs =
Expand All @@ -307,8 +334,8 @@ class _FunctionTypeGenerator extends MemberVisitor1<w.FunctionType, Reference> {
Supertype? supersupertype = initializer.target.enclosingClass.supertype;

if (supersupertype != null) {
w.FunctionType redirectedInitializer = translator.functions
.getFunctionType(initializer.target.initializerReference);
w.FunctionType redirectedInitializer = translator
.signatureForDirectCall(initializer.target.initializerReference);

final int numClassFields = info.getClassFieldTypes().length;
final int numRedirectedContextAndConstructorArgs =
Expand Down Expand Up @@ -374,8 +401,7 @@ class _FunctionTypeGenerator extends MemberVisitor1<w.FunctionType, Reference> {

if (supersupertype != null) {
w.FunctionType superOrRedirectedConstructorBodyType = translator
.functions
.getFunctionType(target.constructorBodyReference);
.signatureForDirectCall(target.constructorBodyReference);

// drop receiver param
inputs += superOrRedirectedConstructorBodyType.inputs.sublist(1);
Expand Down
42 changes: 27 additions & 15 deletions pkg/dart2wasm/lib/translator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -674,8 +674,8 @@ class Translator with KernelNodes {
return tearOffFunctionCache.putIfAbsent(member, () {
assert(member.kind == ProcedureKind.Method);
w.BaseFunction target = functions.getFunction(member.reference);
return getClosure(member.function, target, paramInfoFor(member.reference),
"$member tear-off");
return getClosure(member.function, target,
paramInfoForDirectCall(member.reference), "$member tear-off");
});
}

Expand Down Expand Up @@ -875,23 +875,35 @@ class Translator with KernelNodes {
}
}

w.FunctionType signatureFor(Reference target) {
Member member = target.asMember;
if (member.isInstanceMember) {
return dispatchTable.selectorForTarget(target).signature;
} else {
return functions.getFunctionType(target);
w.FunctionType signatureForDispatchTableCall(Reference target) {
assert(target.asMember.isInstanceMember);
return dispatchTable.selectorForTarget(target).signature;
}

ParameterInfo paramInfoForDispatchTableCall(Reference target) {
assert(target.asMember.isInstanceMember);
return dispatchTable.selectorForTarget(target).paramInfo;
}

w.FunctionType signatureForDirectCall(Reference target) {
if (target.asMember.isInstanceMember) {
final selector = dispatchTable.selectorForTarget(target);
if (selector.targetSet.contains(target)) {
return selector.signature;
}
}
return functions.getFunctionType(target);
}

ParameterInfo paramInfoFor(Reference target) {
Member member = target.asMember;
if (member.isInstanceMember) {
return dispatchTable.selectorForTarget(target).paramInfo;
} else {
return staticParamInfo.putIfAbsent(
target, () => ParameterInfo.fromMember(target));
ParameterInfo paramInfoForDirectCall(Reference target) {
if (target.asMember.isInstanceMember) {
final selector = dispatchTable.selectorForTarget(target);
if (selector.targetSet.contains(target)) {
return selector.paramInfo;
}
}
return staticParamInfo.putIfAbsent(
target, () => ParameterInfo.fromMember(target));
}

w.ValueType preciseThisFor(Member member, {bool nullable = false}) {
Expand Down
4 changes: 2 additions & 2 deletions pkg/dart2wasm/lib/types.dart
Original file line number Diff line number Diff line change
Expand Up @@ -428,8 +428,8 @@ class Types {
b.local_get(operandTemp!);
makeType(codeGen, testedAgainstType);
if (location != null) {
w.FunctionType verifyFunctionType = translator.functions
.getFunctionType(translator.verifyOptimizedTypeCheck.reference);
w.FunctionType verifyFunctionType = translator.signatureForDirectCall(
translator.verifyOptimizedTypeCheck.reference);
translator.constants.instantiateConstant(codeGen.function, b,
StringConstant('$location'), verifyFunctionType.inputs.last);
} else {
Expand Down
53 changes: 53 additions & 0 deletions tests/language/flutter_regession_151473_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'package:expect/expect.dart';

abstract class I {
Base foo();
Base get bar;
}

abstract class A extends I {
Base foo() => Sub1();
Base get bar => Sub1();
}

class B1 extends A {
Base foo() {
print(super.foo());
return Sub2();
}

Base get bar {
print(super.bar);
return Sub2();
}
}

class B2 extends A {
Base foo() {
print(super.foo());
return Sub3();
}

Base get bar {
print(super.bar);
return Sub3();
}
}

abstract class Base {}

class Sub1 extends Base {}

class Sub2 extends Base {}

class Sub3 extends Base {}

main() {
final l = [B1(), B2()];
Expect.isTrue(l[0].foo() is Sub2);
Expect.isTrue(l[0].bar is Sub2);
}

0 comments on commit 0889fdd

Please sign in to comment.