Skip to content

Commit 5ed14b2

Browse files
mkustermannCommit Queue
authored andcommitted
[dart2wasm] Remove dynamic module hacks in dispatch table
The dispatch table was so far getting rows for selectors even though we don't need to call those via dispatch table: * if there's only 0 targets in the module then it's unreachable * if there's only 1 targets in the module then we can call it directly Only if we have >1 targets in the module do we need a dispatch table entry for the selector. So we update the dynamic module updatable function mechanism to handle the * unreachable * direct call * dispatch table call cases just like we do in the normal code generator. Change-Id: I7144771d61fefa09c215e28dd498a4f888c73d22 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/428981 Reviewed-by: Nate Biggs <[email protected]> Reviewed-by: Ömer Ağacan <[email protected]> Commit-Queue: Martin Kustermann <[email protected]>
1 parent ca694e4 commit 5ed14b2

File tree

2 files changed

+88
-38
lines changed

2 files changed

+88
-38
lines changed

pkg/dart2wasm/lib/dispatch_table.dart

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -864,9 +864,11 @@ class DispatchTable {
864864
}
865865

866866
bool _isUsedViaDispatchTableCall(SelectorInfo selector) {
867-
if (selector.isDynamicSubmoduleCallable) return true;
868-
if (selector.callCount == 0) return false;
869-
if (selector.isDynamicSubmoduleOverridable) return true;
867+
// If there's no callers in this module and no callers in dynamic modules then
868+
// there's no need for us to create a dispatch table entry.
869+
if (selector.callCount == 0 && !selector.isDynamicSubmoduleCallable) {
870+
return false;
871+
}
870872

871873
final targets = selector.targets(unchecked: false);
872874

pkg/dart2wasm/lib/dynamic_modules.dart

Lines changed: 83 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import 'dynamic_module_kernel_metadata.dart';
2525
import 'intrinsics.dart' show MemberIntrinsic;
2626
import 'kernel_nodes.dart';
2727
import 'modules.dart';
28+
import 'param_info.dart';
2829
import 'record_class_generator.dart' show dynamicModulesRecordsLibraryUri;
2930
import 'reference_extensions.dart';
3031
import 'target.dart';
@@ -719,25 +720,50 @@ class DynamicModuleInfo {
719720
}
720721

721722
void Function(w.FunctionBuilder) buildSelectorBranch(
722-
Reference target, SelectorInfo mainSelector) {
723+
Reference interfaceTarget, SelectorInfo mainSelector) {
723724
return (w.FunctionBuilder function) {
724-
final localSelector = translator.dispatchTable.selectorForTarget(target);
725-
final localSignature = localSelector.signature;
725+
final localSelector =
726+
translator.dispatchTable.selectorForTarget(interfaceTarget);
726727
final ib = function.body;
727728

728-
final uncheckedOffset = localSelector.targets(unchecked: true).offset;
729-
final checkedOffset = localSelector.targets(unchecked: false).offset;
730-
731-
if (uncheckedOffset == null && checkedOffset == null) {
729+
final uncheckedTargets = localSelector.targets(unchecked: true);
730+
final checkedTargets = localSelector.targets(unchecked: false);
731+
732+
// Whether we use checked+unchecked (or normal) we'll have the same
733+
// class-id ranges - only the actual target `Reference` may be a unchecked
734+
// or checked one.
735+
assert(uncheckedTargets.targetRanges.length ==
736+
checkedTargets.targetRanges.length);
737+
738+
// NOTE: Keep this in sync with
739+
// `code_generator.dart:AstCodeGenerator._virtualCall`.
740+
final bool noTarget = checkedTargets.targetRanges.isEmpty;
741+
final bool directCall = checkedTargets.targetRanges.length == 1;
742+
final callPolymorphicDispatcher =
743+
!directCall && checkedTargets.staticDispatchRanges.isNotEmpty;
744+
// disabled for dyn overridable selectors atm
745+
assert(!callPolymorphicDispatcher);
746+
747+
if (noTarget) {
748+
ib.comment('No targets in local module for ${localSelector.name}');
732749
ib.unreachable();
733750
ib.end();
734751
return;
735752
}
736753

737-
final generalizedMainSignature = _getGeneralizedSignature(mainSelector);
754+
final w.FunctionType localSignature;
755+
final ParameterInfo localParamInfo;
756+
if (directCall) {
757+
final target = checkedTargets.targetRanges.single.target;
758+
localSignature = translator.signatureForDirectCall(target);
759+
localParamInfo = translator.paramInfoForDirectCall(target);
760+
} else {
761+
localSignature = localSelector.signature;
762+
localParamInfo = localSelector.paramInfo;
763+
}
738764

765+
final generalizedMainSignature = _getGeneralizedSignature(mainSelector);
739766
final mainParamInfo = mainSelector.paramInfo;
740-
final localParamInfo = localSelector.paramInfo;
741767

742768
assert(mainParamInfo.takesContextOrReceiver ==
743769
localParamInfo.takesContextOrReceiver);
@@ -799,36 +825,58 @@ class DynamicModuleInfo {
799825
ib, constant, localSignature.inputs[localsIndex]);
800826
}
801827

802-
ib.local_get(ib.locals[function.type.inputs.length - 2]);
803-
if (isSubmodule) {
804-
translator.callReference(translator.scopeClassId.reference, ib);
805-
}
806-
if (checkedOffset == uncheckedOffset) {
807-
if (checkedOffset != 0) {
808-
ib.i32_const(checkedOffset!);
809-
ib.i32_add();
810-
}
811-
} else if (checkedOffset != 0 || uncheckedOffset != 0) {
812-
// Check if the invocation is checked or unchecked and use the
813-
// appropriate offset.
814-
ib.local_get(ib.locals[function.type.inputs.length - 1]);
815-
ib.if_(const [], const [w.NumType.i32]);
816-
if (uncheckedOffset != null) {
817-
ib.i32_const(uncheckedOffset);
828+
if (directCall) {
829+
if (!localSelector.useMultipleEntryPoints) {
830+
final target = checkedTargets.targetRanges.single.target;
831+
ib.invoke(translator.directCallTarget(target));
818832
} else {
819-
ib.unreachable();
833+
final uncheckedTarget = uncheckedTargets.targetRanges.single.target;
834+
final checkedTarget = checkedTargets.targetRanges.single.target;
835+
// Check if the invocation is checked or unchecked and use the
836+
// appropriate offset.
837+
ib.local_get(ib.locals[function.type.inputs.length - 1]);
838+
ib.if_(const [], localSignature.outputs);
839+
ib.invoke(translator.directCallTarget(uncheckedTarget));
840+
ib.else_();
841+
ib.invoke(translator.directCallTarget(checkedTarget));
842+
ib.end();
820843
}
821-
ib.else_();
822-
if (checkedOffset != null) {
823-
ib.i32_const(checkedOffset);
824-
} else {
825-
ib.unreachable();
844+
} else {
845+
ib.local_get(ib.locals[function.type.inputs.length - 2]);
846+
if (isSubmodule) {
847+
translator.callReference(translator.scopeClassId.reference, ib);
826848
}
827-
ib.end();
828-
ib.i32_add();
849+
850+
ib.comment('Local dispatch table call to "${localSelector.name}"');
851+
final uncheckedOffset = uncheckedTargets.offset;
852+
final checkedOffset = checkedTargets.offset;
853+
if (!localSelector.useMultipleEntryPoints) {
854+
if (checkedOffset != 0) {
855+
ib.i32_const(checkedOffset!);
856+
ib.i32_add();
857+
}
858+
} else if (checkedOffset != 0 || uncheckedOffset != 0) {
859+
// Check if the invocation is checked or unchecked and use the
860+
// appropriate offset.
861+
ib.local_get(ib.locals[function.type.inputs.length - 1]);
862+
ib.if_(const [], const [w.NumType.i32]);
863+
if (uncheckedOffset != null) {
864+
ib.i32_const(uncheckedOffset);
865+
} else {
866+
ib.unreachable();
867+
}
868+
ib.else_();
869+
if (checkedOffset != null) {
870+
ib.i32_const(checkedOffset);
871+
} else {
872+
ib.unreachable();
873+
}
874+
ib.end();
875+
ib.i32_add();
876+
}
877+
final table = translator.dispatchTable.getWasmTable(ib.module);
878+
ib.call_indirect(localSignature, table);
829879
}
830-
final table = translator.dispatchTable.getWasmTable(ib.module);
831-
ib.call_indirect(localSignature, table);
832880
translator.convertType(ib, localSignature.outputs.single,
833881
generalizedMainSignature.outputs.single);
834882
ib.end();

0 commit comments

Comments
 (0)