Skip to content

Commit 5f78a23

Browse files
mkustermanncommit-bot@chromium.org
authored andcommitted
[vm/concurrency] Remove usage of current isolate field state from compiler
As part of making the compiler independent of the current isolate (since JITed code will be shared across all isolate) we make the compiler not depend on global field state, with one exception: we preserve an existing optimization that utilizes knowledge whether a global final field was already initialized (if --enable-isolate-groups is turned off) - we do so by asking the isolate group for it's own isolate member. Issue #36097 TEST=Refactoring of existing code. Change-Id: I2f08d854af6102e05e5a1df8d5b66b514d6f3ce4 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/182540 Commit-Queue: Martin Kustermann <[email protected]> Reviewed-by: Alexander Aprelev <[email protected]> Reviewed-by: Ryan Macnak <[email protected]>
1 parent 6ce8d74 commit 5f78a23

File tree

6 files changed

+31
-29
lines changed

6 files changed

+31
-29
lines changed

runtime/vm/compiler/backend/constant_propagator.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -854,8 +854,8 @@ void ConstantPropagator::VisitLoadStaticField(LoadStaticFieldInstr* instr) {
854854
if (!FLAG_fields_may_be_reset) {
855855
const Field& field = instr->field();
856856
ASSERT(field.is_static());
857-
if (field.is_final() && instr->IsFieldInitialized()) {
858-
Instance& obj = Instance::Handle(Z, field.StaticValue());
857+
auto& obj = Instance::Handle(Z);
858+
if (field.is_final() && instr->IsFieldInitialized(&obj)) {
859859
if (obj.IsSmi() || (obj.IsOld() && obj.IsCanonical())) {
860860
SetValue(instr, obj);
861861
return;

runtime/vm/compiler/backend/il.cc

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1135,19 +1135,40 @@ bool LoadStaticFieldInstr::AttributesEqual(Instruction* other) const {
11351135
return field().ptr() == other->AsLoadStaticField()->field().ptr();
11361136
}
11371137

1138-
bool LoadStaticFieldInstr::IsFieldInitialized() const {
1138+
bool LoadStaticFieldInstr::IsFieldInitialized(Instance* field_value) const {
1139+
if (FLAG_fields_may_be_reset) {
1140+
return false;
1141+
}
1142+
1143+
// Since new isolates will be spawned, the JITed code cannot depend on whether
1144+
// global field was initialized when running with --enable-isolate-groups.
1145+
if (IsolateGroup::AreIsolateGroupsEnabled()) return false;
1146+
11391147
const Field& field = this->field();
1140-
return (field.StaticValue() != Object::sentinel().ptr()) &&
1141-
(field.StaticValue() != Object::transition_sentinel().ptr());
1148+
Isolate* only_isolate = IsolateGroup::Current()->FirstIsolate();
1149+
if (only_isolate == nullptr) {
1150+
// This can happen if background compiler executes this code but the mutator
1151+
// is being shutdown and the isolate was already unregistered from the group
1152+
// (and is trying to stop this BG compiler).
1153+
if (field_value != nullptr) {
1154+
*field_value = Object::sentinel().ptr();
1155+
}
1156+
return false;
1157+
}
1158+
if (field_value == nullptr) {
1159+
field_value = &Instance::Handle();
1160+
}
1161+
*field_value = only_isolate->field_table()->At(field.field_id());
1162+
return (field_value->ptr() != Object::sentinel().ptr()) &&
1163+
(field_value->ptr() != Object::transition_sentinel().ptr());
11421164
}
11431165

11441166
Definition* LoadStaticFieldInstr::Canonicalize(FlowGraph* flow_graph) {
11451167
// When precompiling, the fact that a field is currently initialized does not
11461168
// make it safe to omit code that checks if the field needs initialization
11471169
// because the field will be reset so it starts uninitialized in the process
11481170
// running the precompiled code. We must be prepared to reinitialize fields.
1149-
if (calls_initializer() && !FLAG_fields_may_be_reset &&
1150-
IsFieldInitialized()) {
1171+
if (calls_initializer() && IsFieldInitialized()) {
11511172
set_calls_initializer(false);
11521173
}
11531174
return this;

runtime/vm/compiler/backend/il.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5543,7 +5543,7 @@ class LoadStaticFieldInstr : public TemplateDefinition<0, Throws> {
55435543
virtual CompileType ComputeType() const;
55445544

55455545
const Field& field() const { return field_; }
5546-
bool IsFieldInitialized() const;
5546+
bool IsFieldInitialized(Instance* field_value = nullptr) const;
55475547

55485548
bool calls_initializer() const { return calls_initializer_; }
55495549
void set_calls_initializer(bool value) { calls_initializer_ = value; }

runtime/vm/compiler/backend/type_propagator.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1446,9 +1446,9 @@ CompileType LoadStaticFieldInstr::ComputeType() const {
14461446
AbstractType* abstract_type = &AbstractType::ZoneHandle(field.type());
14471447
TraceStrongModeType(this, *abstract_type);
14481448
ASSERT(field.is_static());
1449-
const bool is_initialized = IsFieldInitialized() && !FLAG_fields_may_be_reset;
1449+
auto& obj = Instance::Handle();
1450+
const bool is_initialized = IsFieldInitialized(&obj);
14501451
if (field.is_final() && is_initialized) {
1451-
const Instance& obj = Instance::Handle(field.StaticValue());
14521452
if (!obj.IsNull()) {
14531453
is_nullable = CompileType::kNonNullable;
14541454
cid = obj.GetClassId();

runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -70,20 +70,6 @@ FlowGraph* StreamingFlowGraphBuilder::BuildGraphOfFieldInitializer() {
7070
B->last_used_block_id_, prologue_info);
7171
}
7272

73-
void StreamingFlowGraphBuilder::EvaluateConstFieldValue(const Field& field) {
74-
ASSERT(field.is_const() && field.IsUninitialized());
75-
76-
FieldHelper field_helper(this);
77-
field_helper.ReadUntilExcluding(FieldHelper::kInitializer);
78-
Tag initializer_tag = ReadTag(); // read first part of initializer.
79-
80-
ASSERT(initializer_tag == kSomething);
81-
82-
Instance& value =
83-
Instance::Handle(Z, constant_reader_.ReadConstantExpression());
84-
field.SetStaticValue(value);
85-
}
86-
8773
void StreamingFlowGraphBuilder::SetupDefaultParameterValues() {
8874
intptr_t optional_parameter_count =
8975
parsed_function()->function().NumOptionalParameters();
@@ -1024,10 +1010,6 @@ FlowGraph* StreamingFlowGraphBuilder::BuildGraph() {
10241010
case UntaggedFunction::kImplicitGetter:
10251011
case UntaggedFunction::kImplicitStaticGetter:
10261012
case UntaggedFunction::kImplicitSetter: {
1027-
const Field& field = Field::Handle(Z, function.accessor_field());
1028-
if (field.is_const() && field.IsUninitialized()) {
1029-
EvaluateConstFieldValue(field);
1030-
}
10311013
return B->BuildGraphOfFieldAccessor(function);
10321014
}
10331015
case UntaggedFunction::kFieldInitializer:

runtime/vm/compiler/frontend/kernel_binary_flowgraph.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ class StreamingFlowGraphBuilder : public KernelReaderHelper {
5959

6060
void ParseKernelASTFunction();
6161
void ReadForwardingStubTarget(const Function& function);
62-
void EvaluateConstFieldValue(const Field& field);
6362
void SetupDefaultParameterValues();
6463

6564
FlowGraph* BuildGraphOfFieldInitializer();

0 commit comments

Comments
 (0)