Skip to content

Commit 445d279

Browse files
dcharkescommit-bot@chromium.org
dcharkes
authored and
[email protected]
committed
[vm] Add Reachability Fence Instruction
The reachability fence keeps a value alive and reachable. Required for finalizers: #35770 Design: go/dart-ffi-finalizers (See "Premature Cleanup (Single Object)".) Change-Id: I9742889f0f8d8b15bbcb5dca47f2a4231899dd59 Cq-Include-Trybots: luci.dart.try:vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64-try,app-kernel-linux-debug-x64-try,vm-kernel-linux-debug-ia32-try,vm-kernel-win-debug-x64-try,vm-kernel-win-debug-ia32-try,vm-kernel-precomp-linux-debug-x64-try,vm-dartkb-linux-release-x64-abi-try,vm-kernel-precomp-android-release-arm64-try,vm-kernel-asan-linux-release-x64-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-simarm64-try,vm-kernel-precomp-android-release-arm_x64-try,vm-kernel-precomp-obfuscate-linux-release-x64-try,dart-sdk-linux-try,analyzer-analysis-server-linux-try,analyzer-linux-release-try,front-end-linux-release-x64-try,vm-kernel-precomp-win-release-x64-try,vm-kernel-mac-debug-x64-try,vm-kernel-nnbd-linux-debug-x64-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/136188 Reviewed-by: Vyacheslav Egorov <[email protected]> Reviewed-by: Martin Kustermann <[email protected]> Commit-Queue: Daco Harkes <[email protected]>
1 parent 93c2900 commit 445d279

17 files changed

+302
-6
lines changed

‎pkg/vm/lib/target/vm.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,8 @@ class VmTarget extends Target {
385385
// purposes.
386386
bool allowPlatformPrivateLibraryAccess(Uri importer, Uri imported) =>
387387
super.allowPlatformPrivateLibraryAccess(importer, imported) ||
388-
importer.path.contains('runtime/tests/vm/dart');
388+
importer.path.contains('runtime/tests/vm/dart') ||
389+
importer.path.contains('test-lib');
389390

390391
// TODO(sigmund,ahe): limit this to `dart-ext` libraries only (see
391392
// https://github.com/dart-lang/sdk/issues/29763).

‎runtime/lib/object.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,10 @@ DEFINE_NATIVE_ENTRY(Internal_unsafeCast, 0, 1) {
209209
return arguments->NativeArgAt(0);
210210
}
211211

212+
DEFINE_NATIVE_ENTRY(Internal_reachabilityFence, 0, 1) {
213+
return Object::null();
214+
}
215+
212216
static bool ExtractInterfaceTypeArgs(Zone* zone,
213217
const Class& instance_cls,
214218
const TypeArguments& instance_type_args,

‎runtime/vm/bootstrap_natives.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,7 @@ namespace dart {
330330
V(GrowableList_setLength, 2) \
331331
V(GrowableList_setData, 2) \
332332
V(Internal_unsafeCast, 1) \
333+
V(Internal_reachabilityFence, 1) \
333334
V(Internal_makeListFixedLength, 1) \
334335
V(Internal_makeFixedListUnmodifiable, 1) \
335336
V(Internal_inquireIs64Bit, 0) \

‎runtime/vm/compiler/backend/constant_propagator.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,10 @@ void ConstantPropagator::VisitRedefinition(RedefinitionInstr* instr) {
350350
}
351351
}
352352

353+
void ConstantPropagator::VisitReachabilityFence(ReachabilityFenceInstr* instr) {
354+
// Nothing to do.
355+
}
356+
353357
void ConstantPropagator::VisitCheckArrayBound(CheckArrayBoundInstr* instr) {
354358
// Don't propagate constants through check, since it would eliminate
355359
// the data dependence between the bound check and the load/store.

‎runtime/vm/compiler/backend/il.cc

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4064,6 +4064,21 @@ void RedefinitionInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
40644064
UNREACHABLE();
40654065
}
40664066

4067+
LocationSummary* ReachabilityFenceInstr::MakeLocationSummary(
4068+
Zone* zone,
4069+
bool optimizing) const {
4070+
LocationSummary* summary = new (zone)
4071+
LocationSummary(zone, 1, 0, LocationSummary::ContainsCall::kNoCall);
4072+
// Keep the parameter alive and reachable, in any location.
4073+
summary->set_in(0, Location::Any());
4074+
return summary;
4075+
}
4076+
4077+
void ReachabilityFenceInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
4078+
// No native code, but we rely on the parameter being passed in here so that
4079+
// it stays alive and reachable.
4080+
}
4081+
40674082
LocationSummary* ParameterInstr::MakeLocationSummary(Zone* zone,
40684083
bool optimizing) const {
40694084
UNREACHABLE();

‎runtime/vm/compiler/backend/il.h

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,7 @@ struct InstrAttrs {
367367
M(CatchBlockEntry, kNoGC) \
368368
M(Phi, kNoGC) \
369369
M(Redefinition, kNoGC) \
370+
M(ReachabilityFence, kNoGC) \
370371
M(Parameter, kNoGC) \
371372
M(NativeParameter, kNoGC) \
372373
M(LoadIndexedUnsafe, kNoGC) \
@@ -3296,6 +3297,26 @@ class RedefinitionInstr : public TemplateDefinition<1, NoThrow> {
32963297
DISALLOW_COPY_AND_ASSIGN(RedefinitionInstr);
32973298
};
32983299

3300+
// Keeps the value alive til after this point.
3301+
//
3302+
// The fence cannot be moved.
3303+
class ReachabilityFenceInstr : public TemplateInstruction<1, NoThrow> {
3304+
public:
3305+
explicit ReachabilityFenceInstr(Value* value) { SetInputAt(0, value); }
3306+
3307+
DECLARE_INSTRUCTION(ReachabilityFence)
3308+
3309+
Value* value() const { return inputs_[0]; }
3310+
3311+
virtual bool ComputeCanDeoptimize() const { return false; }
3312+
virtual bool HasUnknownSideEffects() const { return false; }
3313+
3314+
PRINT_OPERANDS_TO_SUPPORT
3315+
3316+
private:
3317+
DISALLOW_COPY_AND_ASSIGN(ReachabilityFenceInstr);
3318+
};
3319+
32993320
class ConstraintInstr : public TemplateDefinition<1, NoThrow> {
33003321
public:
33013322
ConstraintInstr(Value* value, Range* constraint)

‎runtime/vm/compiler/backend/il_printer.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,10 @@ void RedefinitionInstr::PrintOperandsTo(BufferFormatter* f) const {
374374
}
375375
}
376376

377+
void ReachabilityFenceInstr::PrintOperandsTo(BufferFormatter* f) const {
378+
value()->PrintTo(f);
379+
}
380+
377381
void Value::PrintTo(BufferFormatter* f) const {
378382
PrintUse(f, *definition());
379383

‎runtime/vm/compiler/backend/reachability_fence_test.cc

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
// Copyright (c) 2020, 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+
#include <vector>
6+
7+
#include "vm/compiler/backend/il.h"
8+
#include "vm/compiler/backend/il_printer.h"
9+
#include "vm/compiler/backend/il_test_helper.h"
10+
#include "vm/compiler/call_specializer.h"
11+
#include "vm/compiler/compiler_pass.h"
12+
#include "vm/object.h"
13+
#include "vm/unit_test.h"
14+
15+
namespace dart {
16+
17+
ISOLATE_UNIT_TEST_CASE(ReachabilityFence_Simple) {
18+
const char* kScript =
19+
R"(
20+
import 'dart:_internal' show reachabilityFence;
21+
22+
int someGlobal = 0;
23+
24+
class A {
25+
int a;
26+
}
27+
28+
void someFunction(int arg) {
29+
someGlobal += arg;
30+
}
31+
32+
main() {
33+
final object = A()..a = 10;
34+
someFunction(object.a);
35+
reachabilityFence(object);
36+
}
37+
)";
38+
39+
const auto& root_library = Library::Handle(LoadTestScript(kScript));
40+
41+
Invoke(root_library, "main");
42+
43+
const auto& function = Function::Handle(GetFunction(root_library, "main"));
44+
TestPipeline pipeline(function, CompilerPass::kJIT);
45+
FlowGraph* flow_graph = pipeline.RunPasses({});
46+
ASSERT(flow_graph != nullptr);
47+
48+
auto entry = flow_graph->graph_entry()->normal_entry();
49+
EXPECT(entry != nullptr);
50+
51+
// v2 <- AllocateObject(A <not-aliased>) T{A}
52+
// ...
53+
// [use field of object v2]
54+
// ReachabilityFence(v2)
55+
AllocateObjectInstr* allocate_object = nullptr;
56+
ReachabilityFenceInstr* fence = nullptr;
57+
58+
ILMatcher cursor(flow_graph, entry);
59+
RELEASE_ASSERT(cursor.TryMatch({
60+
kMoveGlob,
61+
// Allocate the object.
62+
{kMatchAndMoveAllocateObject, &allocate_object},
63+
kMoveGlob,
64+
// The call.
65+
kMatchAndMoveStoreStaticField,
66+
// The fence should not be moved before the call.
67+
{kMatchAndMoveReachabilityFence, &fence},
68+
}));
69+
70+
EXPECT(fence->value()->definition() == allocate_object);
71+
}
72+
73+
ISOLATE_UNIT_TEST_CASE(ReachabilityFence_Loop) {
74+
const char* kScript =
75+
R"(
76+
import 'dart:_internal' show reachabilityFence;
77+
78+
int someGlobal = 0;
79+
80+
class A {
81+
int a;
82+
}
83+
84+
@pragma('vm:never-inline')
85+
A makeSomeA() {
86+
return A()..a = 10;
87+
}
88+
89+
void someFunction(int arg) {
90+
someGlobal += arg;
91+
}
92+
93+
main() {
94+
final object = makeSomeA();
95+
for(int i = 0; i < 100000; i++) {
96+
someFunction(object.a);
97+
reachabilityFence(object);
98+
}
99+
}
100+
)";
101+
102+
const auto& root_library = Library::Handle(LoadTestScript(kScript));
103+
104+
Invoke(root_library, "main");
105+
106+
const auto& function = Function::Handle(GetFunction(root_library, "main"));
107+
TestPipeline pipeline(function, CompilerPass::kJIT);
108+
FlowGraph* flow_graph = pipeline.RunPasses({});
109+
ASSERT(flow_graph != nullptr);
110+
111+
auto entry = flow_graph->graph_entry()->normal_entry();
112+
EXPECT(entry != nullptr);
113+
114+
StaticCallInstr* object = nullptr;
115+
LoadFieldInstr* field_load = nullptr;
116+
ReachabilityFenceInstr* fence = nullptr;
117+
118+
ILMatcher cursor(flow_graph, entry);
119+
RELEASE_ASSERT(cursor.TryMatch(
120+
{
121+
// Get the object from some method
122+
{kMatchAndMoveStaticCall, &object},
123+
// Load the field outside the loop.
124+
{kMatchAndMoveLoadField, &field_load},
125+
// Go into the loop.
126+
kMatchAndMoveBranchTrue,
127+
// The fence should not be moved outside of the loop.
128+
{kMatchAndMoveReachabilityFence, &fence},
129+
},
130+
/*insert_before=*/kMoveGlob));
131+
132+
EXPECT(field_load->instance()->definition() == object);
133+
EXPECT(fence->value()->definition() == object);
134+
}
135+
136+
ISOLATE_UNIT_TEST_CASE(ReachabilityFence_NoCanonicalize) {
137+
const char* kScript =
138+
R"(
139+
import 'dart:_internal' show reachabilityFence;
140+
141+
int someGlobal = 0;
142+
143+
class A {
144+
int a;
145+
}
146+
147+
@pragma('vm:never-inline')
148+
A makeSomeA() {
149+
return A()..a = 10;
150+
}
151+
152+
void someFunction(int arg) {
153+
someGlobal += arg;
154+
}
155+
156+
main() {
157+
final object = makeSomeA();
158+
reachabilityFence(object);
159+
for(int i = 0; i < 100000; i++) {
160+
someFunction(object.a);
161+
reachabilityFence(object);
162+
}
163+
reachabilityFence(object);
164+
reachabilityFence(object);
165+
}
166+
)";
167+
168+
const auto& root_library = Library::Handle(LoadTestScript(kScript));
169+
170+
Invoke(root_library, "main");
171+
172+
const auto& function = Function::Handle(GetFunction(root_library, "main"));
173+
TestPipeline pipeline(function, CompilerPass::kJIT);
174+
FlowGraph* flow_graph = pipeline.RunPasses({});
175+
ASSERT(flow_graph != nullptr);
176+
177+
auto entry = flow_graph->graph_entry()->normal_entry();
178+
EXPECT(entry != nullptr);
179+
180+
StaticCallInstr* object = nullptr;
181+
ReachabilityFenceInstr* fence1 = nullptr;
182+
ReachabilityFenceInstr* fence2 = nullptr;
183+
ReachabilityFenceInstr* fence3 = nullptr;
184+
ReachabilityFenceInstr* fence4 = nullptr;
185+
186+
ILMatcher cursor(flow_graph, entry);
187+
RELEASE_ASSERT(cursor.TryMatch(
188+
{
189+
{kMatchAndMoveStaticCall, &object},
190+
{kMatchAndMoveReachabilityFence, &fence1},
191+
kMatchAndMoveBranchTrue,
192+
{kMatchAndMoveReachabilityFence, &fence2},
193+
kMatchAndMoveBranchFalse,
194+
{kMatchAndMoveReachabilityFence, &fence3},
195+
{kMatchAndMoveReachabilityFence, &fence4},
196+
},
197+
/*insert_before=*/kMoveGlob));
198+
199+
EXPECT(fence1->value()->definition() == object);
200+
EXPECT(fence2->value()->definition() == object);
201+
EXPECT(fence3->value()->definition() == object);
202+
EXPECT(fence4->value()->definition() == object);
203+
}
204+
205+
} // namespace dart

‎runtime/vm/compiler/backend/redundancy_elimination.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3690,7 +3690,7 @@ static bool CanEliminateInstruction(Instruction* current,
36903690
ASSERT(current->GetBlock() == block);
36913691
if (MayHaveVisibleEffect(current) || current->CanDeoptimize() ||
36923692
current == block->last_instruction() || current->IsMaterializeObject() ||
3693-
current->IsCheckStackOverflow()) {
3693+
current->IsCheckStackOverflow() || current->IsReachabilityFence()) {
36943694
return false;
36953695
}
36963696
return true;

‎runtime/vm/compiler/compiler_sources.gni

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ compiler_sources_tests = [
185185
"backend/locations_helpers_test.cc",
186186
"backend/loops_test.cc",
187187
"backend/range_analysis_test.cc",
188+
"backend/reachability_fence_test.cc",
188189
"backend/redundancy_elimination_test.cc",
189190
"backend/sexpression_test.cc",
190191
"backend/slot_test.cc",

‎runtime/vm/compiler/frontend/base_flow_graph_builder.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -555,6 +555,12 @@ Fragment BaseFlowGraphBuilder::RedefinitionWithType(const AbstractType& type) {
555555
return Fragment(redefinition);
556556
}
557557

558+
Fragment BaseFlowGraphBuilder::ReachabilityFence() {
559+
Fragment instructions;
560+
instructions <<= new (Z) ReachabilityFenceInstr(Pop());
561+
return instructions;
562+
}
563+
558564
Fragment BaseFlowGraphBuilder::StoreStaticField(TokenPosition position,
559565
const Field& field) {
560566
return Fragment(

‎runtime/vm/compiler/frontend/base_flow_graph_builder.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@ class BaseFlowGraphBuilder {
198198
StoreInstanceFieldInstr::Kind::kOther);
199199
Fragment LoadStaticField(const Field& field);
200200
Fragment RedefinitionWithType(const AbstractType& type);
201+
Fragment ReachabilityFence();
201202
Fragment StoreStaticField(TokenPosition position, const Field& field);
202203
Fragment StoreIndexed(classid_t class_id);
203204
// Takes a [class_id] valid for StoreIndexed.

0 commit comments

Comments
 (0)