Skip to content

Commit 286b123

Browse files
mralephcommit-bot@chromium.org
authored andcommitted
[vm/kernel/aot] Remove type checks from non-dynamically-invoked implicit setters
This applies optimization from d117760 to implicit setters. (This CL also reformats kernel_binary_flowgraph.cc because some previous CL was pushed without proper formatting) Bug: #31798 Change-Id: I6f1590bfd40e36b972b857c49d6ce8435bb25187 Reviewed-on: https://dart-review.googlesource.com/41300 Commit-Queue: Vyacheslav Egorov <[email protected]> Reviewed-by: Martin Kustermann <[email protected]>
1 parent 7ca4792 commit 286b123

File tree

3 files changed

+63
-17
lines changed

3 files changed

+63
-17
lines changed

pkg/vm/lib/transformations/no_dynamic_invocations_annotator.dart

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ class Selector {
2424

2525
Selector(this.action, this.target);
2626

27+
Selector.invoke(Name target) : this(Action.invoke, target);
28+
Selector.get(Name target) : this(Action.get, target);
29+
Selector.set(Name target) : this(Action.set, target);
30+
2731
bool operator ==(other) {
2832
return other is Selector &&
2933
other.action == this.action &&
@@ -46,7 +50,7 @@ class Selector {
4650
}
4751
}
4852

49-
class NoDynamicInvocationsAnnotator extends RecursiveVisitor<Null> {
53+
class NoDynamicInvocationsAnnotator {
5054
final Set<Selector> _dynamicSelectors;
5155
final ProcedureAttributesMetadataRepository _metadata;
5256

@@ -56,17 +60,45 @@ class NoDynamicInvocationsAnnotator extends RecursiveVisitor<Null> {
5660
program.addMetadataRepository(_metadata);
5761
}
5862

59-
@override
63+
visitProgram(Program program) {
64+
for (var library in program.libraries) {
65+
for (var klass in library.classes) {
66+
visitClass(klass);
67+
}
68+
}
69+
}
70+
71+
visitClass(Class node) {
72+
for (var member in node.members) {
73+
if (member is Procedure) {
74+
visitProcedure(member);
75+
} else if (member is Field) {
76+
visitField(member);
77+
}
78+
}
79+
}
80+
81+
visitField(Field node) {
82+
if (node.isStatic || node.name.name == 'call') {
83+
return;
84+
}
85+
86+
if (!_dynamicSelectors.contains(new Selector.set(node.name))) {
87+
_metadata.mapping[node] =
88+
const ProcedureAttributesMetadata.noDynamicInvocations();
89+
}
90+
}
91+
6092
visitProcedure(Procedure node) {
6193
if (node.isStatic || node.name.name == 'call') {
6294
return;
6395
}
6496

6597
Selector selector;
6698
if (node.kind == ProcedureKind.Method) {
67-
selector = new Selector(Action.invoke, node.name);
99+
selector = new Selector.invoke(node.name);
68100
} else if (node.kind == ProcedureKind.Setter) {
69-
selector = new Selector(Action.set, node.name);
101+
selector = new Selector.set(node.name);
70102
} else {
71103
return;
72104
}
@@ -92,7 +124,7 @@ class DynamicSelectorsCollector extends RecursiveVisitor<Null> {
92124
super.visitMethodInvocation(node);
93125

94126
if (node.dispatchCategory == DispatchCategory.dynamicDispatch) {
95-
dynamicSelectors.add(new Selector(Action.invoke, node.name));
127+
dynamicSelectors.add(new Selector.invoke(node.name));
96128
}
97129
}
98130

@@ -101,7 +133,7 @@ class DynamicSelectorsCollector extends RecursiveVisitor<Null> {
101133
super.visitPropertyGet(node);
102134

103135
if (node.dispatchCategory == DispatchCategory.dynamicDispatch) {
104-
dynamicSelectors.add(new Selector(Action.get, node.name));
136+
dynamicSelectors.add(new Selector.get(node.name));
105137
}
106138
}
107139

@@ -110,7 +142,7 @@ class DynamicSelectorsCollector extends RecursiveVisitor<Null> {
110142
super.visitPropertySet(node);
111143

112144
if (node.dispatchCategory == DispatchCategory.dynamicDispatch) {
113-
dynamicSelectors.add(new Selector(Action.set, node.name));
145+
dynamicSelectors.add(new Selector.set(node.name));
114146
}
115147
}
116148
}

runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1007,8 +1007,8 @@ ScopeBuildingResult* StreamingScopeBuilder::BuildScopes() {
10071007
VisitNode();
10081008
break;
10091009
}
1010-
bool is_setter = function.IsImplicitSetterFunction();
1011-
bool is_method = !function.IsStaticFunction();
1010+
const bool is_setter = function.IsImplicitSetterFunction();
1011+
const bool is_method = !function.IsStaticFunction();
10121012
intptr_t pos = 0;
10131013
if (is_method) {
10141014
Class& klass = Class::Handle(Z, function.Owner());
@@ -1025,6 +1025,16 @@ ScopeBuildingResult* StreamingScopeBuilder::BuildScopes() {
10251025
Symbols::Value(),
10261026
AbstractType::ZoneHandle(Z, function.ParameterTypeAt(pos)));
10271027
scope_->InsertParameterAt(pos++, result_->setter_value);
1028+
1029+
if (is_method && !attrs.has_dynamic_invocations) {
1030+
FieldHelper field_helper(builder_);
1031+
field_helper.ReadUntilIncluding(FieldHelper::kFlags);
1032+
1033+
if (!field_helper.IsGenericCovariantImpl()) {
1034+
result_->setter_value->set_type_check_mode(
1035+
LocalVariable::kTypeCheckedByCaller);
1036+
}
1037+
}
10281038
}
10291039
break;
10301040
}
@@ -5842,9 +5852,9 @@ Fragment StreamingFlowGraphBuilder::StaticCall(
58425852
ICData::RebindRule rebind_rule,
58435853
const InferredTypeMetadata* result_type,
58445854
intptr_t type_args_count) {
5845-
return flow_graph_builder_->StaticCall(
5846-
position, target, argument_count, argument_names, rebind_rule,
5847-
result_type, type_args_count);
5855+
return flow_graph_builder_->StaticCall(position, target, argument_count,
5856+
argument_names, rebind_rule,
5857+
result_type, type_args_count);
58485858
}
58495859

58505860
Fragment StreamingFlowGraphBuilder::InstanceCall(
@@ -6928,7 +6938,7 @@ Fragment StreamingFlowGraphBuilder::BuildMethodInvocation(TokenPosition* p) {
69286938
Fragment StreamingFlowGraphBuilder::BuildDirectMethodInvocation(
69296939
TokenPosition* p) {
69306940
const intptr_t offset = ReaderOffset() - 1; // Include the tag.
6931-
TokenPosition position = ReadPosition(); // read offset.
6941+
TokenPosition position = ReadPosition(); // read offset.
69326942
if (p != NULL) *p = position;
69336943

69346944
const InferredTypeMetadata result_type =
@@ -6991,9 +7001,9 @@ Fragment StreamingFlowGraphBuilder::BuildDirectMethodInvocation(
69917001
&positional_argument_count); // read arguments.
69927002
++argument_count;
69937003

6994-
return instructions +
6995-
StaticCall(position, target, argument_count, argument_names,
6996-
ICData::kNoRebind, &result_type, type_args_len);
7004+
return instructions + StaticCall(position, target, argument_count,
7005+
argument_names, ICData::kNoRebind,
7006+
&result_type, type_args_len);
69977007
}
69987008

69997009
Fragment StreamingFlowGraphBuilder::BuildSuperMethodInvocation(
@@ -7143,7 +7153,7 @@ Fragment StreamingFlowGraphBuilder::BuildSuperMethodInvocation(
71437153
Fragment StreamingFlowGraphBuilder::BuildStaticInvocation(bool is_const,
71447154
TokenPosition* p) {
71457155
const intptr_t offset = ReaderOffset() - 1; // Include the tag.
7146-
TokenPosition position = ReadPosition(); // read position.
7156+
TokenPosition position = ReadPosition(); // read position.
71477157
if (p != NULL) *p = position;
71487158

71497159
const InferredTypeMetadata result_type =

runtime/vm/compiler/frontend/kernel_binary_flowgraph.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ class FieldHelper {
174174
kFinal = 1 << 0,
175175
kConst = 1 << 1,
176176
kStatic = 1 << 2,
177+
kIsGenericCovariantImpl = 1 << 6,
177178
kIsGenericCovariantInterface = 1 << 7
178179
};
179180

@@ -197,6 +198,9 @@ class FieldHelper {
197198
bool IsConst() { return (flags_ & kConst) != 0; }
198199
bool IsFinal() { return (flags_ & kFinal) != 0; }
199200
bool IsStatic() { return (flags_ & kStatic) != 0; }
201+
bool IsGenericCovariantImpl() {
202+
return (flags_ & kIsGenericCovariantImpl) != 0;
203+
}
200204
bool IsGenericCovariantInterface() {
201205
return (flags_ & kIsGenericCovariantInterface) != 0;
202206
}

0 commit comments

Comments
 (0)