Skip to content

Commit 293cc64

Browse files
alexmarkovCommit Queue
authored and
Commit Queue
committed
[vm/aot/tfa] Whole-program propagation of closure values
This change adds propagation of closure values to TFA. For now, inferred closure values are used only when they are constant (tear-off of a static method or a constructor). Arbitrary closures can now be referenced from unrelated members via closure-id metadata. In addition, this change fixes an incorrect stack trace when an implicit closure (tear-off) was propagated and its call was inlined. Inlining interval was not recorded because of the missing source position of a call to a target method within implicit closure. TEST=pkg/vm/testcases/transformations/type_flow/transformer/closures.dart Issue: #39692 Change-Id: I3590da91b6057e0b55a8614382dba1bbcc267b39 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/325447 Commit-Queue: Alexander Markov <[email protected]> Reviewed-by: Martin Kustermann <[email protected]> Reviewed-by: Slava Egorov <[email protected]>
1 parent 3f3def4 commit 293cc64

36 files changed

+677
-111
lines changed

pkg/dart2wasm/lib/target.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ class WasmTarget extends Target {
102102
Class? _wasmImmutableSet;
103103
Class? _oneByteString;
104104
Class? _twoByteString;
105+
Class? _closure;
105106
Map<String, Class>? _nativeClasses;
106107

107108
@override
@@ -479,6 +480,11 @@ class WasmTarget extends Target {
479480
coreTypes.index.getClass('dart:_string', 'OneByteString');
480481
}
481482

483+
@override
484+
Class concreteClosureClass(CoreTypes coreTypes) {
485+
return _closure ??= coreTypes.index.getClass('dart:core', '_Closure');
486+
}
487+
482488
@override
483489
bool isSupportedPragma(String pragmaName) => pragmaName.startsWith("wasm:");
484490

pkg/kernel/lib/target/targets.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,7 @@ abstract class Target {
515515
Class? concreteConstMapLiteralClass(CoreTypes coreTypes) => null;
516516
Class? concreteSetLiteralClass(CoreTypes coreTypes) => null;
517517
Class? concreteConstSetLiteralClass(CoreTypes coreTypes) => null;
518+
Class? concreteClosureClass(CoreTypes coreTypes) => null;
518519
Class getRecordImplementationClass(CoreTypes coreTypes,
519520
int numPositionalFields, List<String> namedFields) =>
520521
throw UnsupportedError('Target.getRecordImplementationClass');

pkg/vm/bin/dump_kernel.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import 'package:kernel/kernel.dart' show Component, writeComponentToText;
88
import 'package:kernel/binary/ast_from_binary.dart'
99
show BinaryBuilderWithMetadata;
1010

11+
import 'package:vm/metadata/closure_id.dart' show ClosureIdMetadataRepository;
1112
import 'package:vm/metadata/direct_call.dart' show DirectCallMetadataRepository;
1213
import 'package:vm/metadata/inferred_type.dart'
1314
show InferredTypeMetadataRepository, InferredArgTypeMetadataRepository;
@@ -50,6 +51,7 @@ main(List<String> arguments) async {
5051
component.addMetadataRepository(new UnreachableNodeMetadataRepository());
5152
component.addMetadataRepository(new CallSiteAttributesMetadataRepository());
5253
component.addMetadataRepository(new LoadingUnitsMetadataRepository());
54+
component.addMetadataRepository(new ClosureIdMetadataRepository());
5355

5456
final List<int> bytes = new File(input).readAsBytesSync();
5557
new BinaryBuilderWithMetadata(bytes).readComponent(component);

pkg/vm/lib/metadata/closure_id.dart

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:kernel/ast.dart';
6+
7+
/// Repository for persistent closure IDs.
8+
class ClosureIdMetadataRepository extends MetadataRepository<int> {
9+
static const String repositoryTag = 'vm.closure-id';
10+
11+
@override
12+
String get tag => repositoryTag;
13+
14+
// For a LocalFunction: id within an enclosing Member,
15+
// with 0 reserved for the tear-off of the Member.
16+
//
17+
// For a Member: number of nested closures.
18+
@override
19+
final Map<TreeNode, int> mapping = {};
20+
21+
@override
22+
void writeToBinary(int metadata, Node node, BinarySink sink) {
23+
sink.writeUInt30(metadata);
24+
}
25+
26+
@override
27+
int readFromBinary(Node node, BinarySource source) {
28+
return source.readUInt30();
29+
}
30+
31+
/// Return closure ID within the enclosing [Member].
32+
///
33+
/// Closures should be indexed within enclosing [Member]
34+
/// using [indexClosures].
35+
int getClosureId(LocalFunction closure) => mapping[closure]!;
36+
37+
/// Assign IDs for all closures within [member].
38+
void indexClosures(Member member) {
39+
if (mapping.containsKey(member)) {
40+
return;
41+
}
42+
_ClosureIndexer indexer = _ClosureIndexer(this, member);
43+
member.accept(indexer);
44+
mapping[member] = indexer.index - _ClosureIndexer.firstClosureIndex;
45+
}
46+
}
47+
48+
class _ClosureIndexer extends RecursiveVisitor<void> {
49+
// Zero is reserved for tear-offs.
50+
static int firstClosureIndex = 1;
51+
52+
final ClosureIdMetadataRepository _repository;
53+
final Member member;
54+
int index = firstClosureIndex;
55+
56+
_ClosureIndexer(this._repository, this.member);
57+
58+
@override
59+
void visitFunctionDeclaration(FunctionDeclaration node) =>
60+
_visitLocalFunction(node);
61+
62+
@override
63+
void visitFunctionExpression(FunctionExpression node) =>
64+
_visitLocalFunction(node);
65+
66+
void _visitLocalFunction(LocalFunction node) {
67+
assert(index > 0);
68+
_repository.mapping[node] = index++;
69+
node.visitChildren(this);
70+
}
71+
}

pkg/vm/lib/metadata/inferred_type.dart

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import 'package:kernel/src/printer.dart';
1111
class InferredType {
1212
final Reference? _concreteClassReference;
1313
final Constant? _constantValue;
14+
final Reference? _closureMemberReference;
15+
final int _closureId;
1416
final int _flags;
1517

1618
static const int flagNullable = 1 << 0;
@@ -19,10 +21,14 @@ class InferredType {
1921
// For invocations: whether to use the unchecked entry-point.
2022
static const int flagSkipCheck = 1 << 2;
2123

24+
// Contains inferred constant value.
2225
static const int flagConstant = 1 << 3;
2326

2427
static const int flagReceiverNotInt = 1 << 4;
2528

29+
// Contains inferred closure value.
30+
static const int flagClosure = 1 << 5;
31+
2632
// Entire list may be null if no type arguments were inferred.
2733
// Will always be null if `concreteClass` is null.
2834
//
@@ -33,31 +39,44 @@ class InferredType {
3339
// argument (in the runtime type) is always exactly a particular `DartType`.
3440
final List<DartType?>? exactTypeArguments;
3541

36-
InferredType(
37-
Class? concreteClass, bool nullable, bool isInt, Constant? constantValue,
42+
InferredType(Class? concreteClass, bool nullable, bool isInt,
43+
Constant? constantValue, Member? closureMember, int closureId,
3844
{List<DartType?>? exactTypeArguments,
3945
bool skipCheck = false,
4046
bool receiverNotInt = false})
4147
: this._byReference(
4248
concreteClass?.reference,
4349
constantValue,
50+
closureMember?.reference,
51+
closureId,
4452
(nullable ? flagNullable : 0) |
4553
(isInt ? flagInt : 0) |
4654
(skipCheck ? flagSkipCheck : 0) |
4755
(constantValue != null ? flagConstant : 0) |
48-
(receiverNotInt ? flagReceiverNotInt : 0),
56+
(receiverNotInt ? flagReceiverNotInt : 0) |
57+
(closureMember != null ? flagClosure : 0),
4958
exactTypeArguments);
5059

51-
InferredType._byReference(this._concreteClassReference, this._constantValue,
52-
this._flags, this.exactTypeArguments) {
60+
InferredType._byReference(
61+
this._concreteClassReference,
62+
this._constantValue,
63+
this._closureMemberReference,
64+
this._closureId,
65+
this._flags,
66+
this.exactTypeArguments) {
5367
assert(exactTypeArguments == null || _concreteClassReference != null);
5468
assert(_constantValue == null || _concreteClassReference != null);
69+
assert(_closureMemberReference == null || _concreteClassReference != null);
70+
assert(_closureId >= 0);
5571
}
5672

5773
Class? get concreteClass => _concreteClassReference?.asClass;
5874

5975
Constant? get constantValue => _constantValue;
6076

77+
Member? get closureMember => _closureMemberReference?.asMember;
78+
int get closureId => _closureId;
79+
6180
bool get nullable => (_flags & flagNullable) != 0;
6281
bool get isInt => (_flags & flagInt) != 0;
6382
bool get skipCheck => (_flags & flagSkipCheck) != 0;
@@ -96,6 +115,10 @@ class InferredType {
96115
if (receiverNotInt) {
97116
buf.write(' (receiver not int)');
98117
}
118+
if (closureMember != null) {
119+
buf.write(
120+
' (closure ${closureId} in ${closureMember!.toText(astTextStrategyForTesting)})');
121+
}
99122
return buf.toString();
100123
}
101124
}
@@ -116,10 +139,16 @@ class InferredTypeMetadataRepository extends MetadataRepository<InferredType> {
116139
// them for optimizations.
117140
sink.writeNullAllowedCanonicalNameReference(
118141
metadata.concreteClass?.reference);
142+
final flags = metadata._flags;
119143
sink.writeByte(metadata._flags);
120-
if (metadata.constantValue != null) {
144+
if ((flags & InferredType.flagConstant) != 0) {
121145
sink.writeConstantReference(metadata.constantValue!);
122146
}
147+
if ((flags & InferredType.flagClosure) != 0) {
148+
sink.writeNullAllowedCanonicalNameReference(
149+
metadata.closureMember!.reference);
150+
sink.writeUInt30(metadata.closureId);
151+
}
123152
}
124153

125154
@override
@@ -132,8 +161,13 @@ class InferredTypeMetadataRepository extends MetadataRepository<InferredType> {
132161
final constantValue = (flags & InferredType.flagConstant) != 0
133162
? source.readConstantReference()
134163
: null;
135-
return new InferredType._byReference(
136-
concreteClassReference, constantValue, flags, null);
164+
final closureMemberReference = (flags & InferredType.flagClosure) != 0
165+
? source.readNullableCanonicalNameReference()!.reference
166+
: null;
167+
final closureId =
168+
(flags & InferredType.flagClosure) != 0 ? source.readUInt30() : 0;
169+
return new InferredType._byReference(concreteClassReference, constantValue,
170+
closureMemberReference, closureId, flags, null);
137171
}
138172
}
139173

pkg/vm/lib/target/vm.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ class VmTarget extends Target {
3939
Class? _twoByteString;
4040
Class? _smi;
4141
Class? _double; // _Double, not double.
42+
Class? _closure;
4243
Class? _syncStarIterable;
4344

4445
VmTarget(this.flags);
@@ -490,6 +491,11 @@ class VmTarget extends Target {
490491
coreTypes.index.getClass('dart:core', '_OneByteString');
491492
}
492493

494+
@override
495+
Class concreteClosureClass(CoreTypes coreTypes) {
496+
return _closure ??= coreTypes.index.getClass('dart:core', '_Closure');
497+
}
498+
493499
@override
494500
Class? concreteAsyncResultClass(CoreTypes coreTypes) =>
495501
coreTypes.futureImplClass;

pkg/vm/lib/transformations/call_site_annotator.dart

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -83,19 +83,6 @@ class AnnotateWithStaticTypes extends RecursiveVisitor {
8383
}
8484
}
8585

86-
@override
87-
visitFunctionInvocation(FunctionInvocation node) {
88-
super.visitFunctionInvocation(node);
89-
90-
final DartType receiverType =
91-
node.receiver.getStaticType(_staticTypeContext!);
92-
if (receiverType is FunctionType &&
93-
node.kind == FunctionAccessKind.Function) {
94-
throw 'Node ${node.runtimeType}: $node at ${node.location} has receiver'
95-
' static type $receiverType, but kind ${node.kind}';
96-
}
97-
}
98-
9986
@override
10087
visitEqualsCall(EqualsCall node) {
10188
super.visitEqualsCall(node);

pkg/vm/lib/transformations/type_flow/analysis.dart

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,9 +315,29 @@ final class _DirectInvocation extends _Invocation {
315315
if (selector.callKind == CallKind.PropertyGet) {
316316
// Tear-off.
317317
// TODO(alexmarkov): capture receiver type
318-
assert((member is Procedure) && !member.isGetter && !member.isSetter);
318+
assert((member is Procedure) &&
319+
!member.isGetter &&
320+
!member.isSetter &&
321+
!member.isFactory &&
322+
!member.isAbstract);
319323
typeFlowAnalysis.addRawCall(new DirectSelector(member));
320324
typeFlowAnalysis._tearOffTaken.add(member);
325+
final Class? concreteClass = typeFlowAnalysis.target
326+
.concreteClosureClass(typeFlowAnalysis.coreTypes);
327+
if (concreteClass != null) {
328+
if (!member.isInstanceMember) {
329+
return typeFlowAnalysis
330+
.addAllocatedClass(concreteClass)
331+
.cls
332+
.constantConcreteType(
333+
StaticTearOffConstant(member as Procedure));
334+
} else {
335+
return typeFlowAnalysis
336+
.addAllocatedClass(concreteClass)
337+
.cls
338+
.closureConcreteType(member, null);
339+
}
340+
}
321341
return nullableAnyType;
322342
} else {
323343
// Call via getter.

pkg/vm/lib/transformations/type_flow/signature_shaking.dart

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ class _ParameterInfo {
182182
// constant value in every implementation. The constant value inferred does
183183
// not have to be the same across implementations, as it is specialized in
184184
// each implementation individually.
185-
if (!(type is ConcreteType && type.constant != null ||
185+
if (!(type is ConcreteType && type.attributes?.constant != null ||
186186
type is NullableType && type.baseType is EmptyType)) {
187187
isConstant = false;
188188
}
@@ -372,7 +372,7 @@ class _Transform extends RecursiveVisitor {
372372
if (param.isConstant) {
373373
Type type = shaker.typeFlowAnalysis.argumentType(member, variable)!;
374374
if (type is ConcreteType) {
375-
value = type.constant!;
375+
value = type.attributes!.constant!;
376376
} else {
377377
assert(type is NullableType && type.baseType is EmptyType);
378378
value = NullConstant();
@@ -499,7 +499,8 @@ class _Transform extends RecursiveVisitor {
499499
void visitVariableGet(VariableGet node) {
500500
Constant? constantValue = eliminatedParams[node.variable];
501501
if (constantValue != null) {
502-
node.replaceWith(ConstantExpression(constantValue));
502+
node.replaceWith(ConstantExpression(
503+
constantValue, constantValue.getType(typeContext)));
503504
}
504505
}
505506

0 commit comments

Comments
 (0)