Skip to content

Commit

Permalink
[vm/concurrency] Move osr/field-guard related state from Isolate to…
Browse files Browse the repository at this point in the history
… `IsolateGroup`

As part of making the compiler independent of `Isolate` we have to move
various state from `Isolate` to `IsolateGroup`.

This CL moves osr/field-guard related state.

TEST=Pure refactoring - relying on existing test coverage.

Issue #36097

Change-Id: Id943891476f892f64f2a74952681270d012a4693
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/177125
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
  • Loading branch information
mkustermann authored and commit-bot@chromium.org committed Jan 6, 2021
1 parent 12d7707 commit 897b1a2
Show file tree
Hide file tree
Showing 22 changed files with 79 additions and 70 deletions.
4 changes: 2 additions & 2 deletions runtime/vm/class_finalizer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -934,13 +934,13 @@ void ClassFinalizer::FinalizeMemberTypes(const Class& cls) {
// instance method.

// Finalize type of fields and check for conflicts in super classes.
Isolate* isolate = Isolate::Current();
auto isolate_group = IsolateGroup::Current();
Zone* zone = Thread::Current()->zone();
Array& array = Array::Handle(zone, cls.fields());
Field& field = Field::Handle(zone);
AbstractType& type = AbstractType::Handle(zone);
const intptr_t num_fields = array.Length();
const bool track_exactness = isolate->use_field_guards();
const bool track_exactness = isolate_group->use_field_guards();
for (intptr_t i = 0; i < num_fields; i++) {
field ^= array.At(i);
type = field.type();
Expand Down
2 changes: 1 addition & 1 deletion runtime/vm/clustered_snapshot.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1134,7 +1134,7 @@ class FieldDeserializationCluster : public DeserializationCluster {

void PostLoad(Deserializer* d, const Array& refs, bool is_canonical) {
Field& field = Field::Handle(d->zone());
if (!Isolate::Current()->use_field_guards()) {
if (!IsolateGroup::Current()->use_field_guards()) {
for (intptr_t i = start_index_; i < stop_index_; i++) {
field ^= refs.At(i);
field.set_guarded_cid_unsafe(kDynamicCid);
Expand Down
1 change: 1 addition & 0 deletions runtime/vm/compiler/backend/flow_graph.h
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ class FlowGraph : public ZoneAllocated {
Thread* thread() const { return thread_; }
Zone* zone() const { return thread()->zone(); }
Isolate* isolate() const { return thread()->isolate(); }
IsolateGroup* isolate_group() const { return thread()->isolate_group(); }

intptr_t max_block_id() const { return max_block_id_; }
void set_max_block_id(intptr_t id) { max_block_id_ = id; }
Expand Down
3 changes: 2 additions & 1 deletion runtime/vm/compiler/backend/flow_graph_compiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,8 @@ bool FlowGraphCompiler::CanOptimizeFunction() const {
}

bool FlowGraphCompiler::CanOSRFunction() const {
return isolate()->use_osr() && CanOptimizeFunction() && !is_optimizing();
return isolate_group()->use_osr() && CanOptimizeFunction() &&
!is_optimizing();
}

void FlowGraphCompiler::InsertBSSRelocation(BSS::Relocation reloc) {
Expand Down
6 changes: 3 additions & 3 deletions runtime/vm/compiler/backend/il_arm.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3726,7 +3726,7 @@ class CheckStackOverflowSlowPath
: TemplateSlowPathCode(instruction) {}

virtual void EmitNativeCode(FlowGraphCompiler* compiler) {
if (compiler->isolate()->use_osr() && osr_entry_label()->IsLinked()) {
if (compiler->isolate_group()->use_osr() && osr_entry_label()->IsLinked()) {
const Register value = instruction()->locs()->temp(0).reg();
__ Comment("CheckStackOverflowSlowPathOsr");
__ Bind(osr_entry_label());
Expand Down Expand Up @@ -3766,7 +3766,7 @@ class CheckStackOverflowSlowPath
kStackOverflowRuntimeEntry, kNumSlowPathArgs, instruction()->locs());
}

if (compiler->isolate()->use_osr() && !compiler->is_optimizing() &&
if (compiler->isolate_group()->use_osr() && !compiler->is_optimizing() &&
instruction()->in_loop()) {
// In unoptimized code, record loop stack checks as possible OSR entries.
compiler->AddCurrentDescriptor(PcDescriptorsLayout::kOsrEntry,
Expand All @@ -3781,7 +3781,7 @@ class CheckStackOverflowSlowPath
}

compiler::Label* osr_entry_label() {
ASSERT(Isolate::Current()->use_osr());
ASSERT(IsolateGroup::Current()->use_osr());
return &osr_entry_label_;
}

Expand Down
6 changes: 3 additions & 3 deletions runtime/vm/compiler/backend/il_arm64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3240,7 +3240,7 @@ class CheckStackOverflowSlowPath

virtual void EmitNativeCode(FlowGraphCompiler* compiler) {
auto locs = instruction()->locs();
if (compiler->isolate()->use_osr() && osr_entry_label()->IsLinked()) {
if (compiler->isolate_group()->use_osr() && osr_entry_label()->IsLinked()) {
const Register value = locs->temp(0).reg();
__ Comment("CheckStackOverflowSlowPathOsr");
__ Bind(osr_entry_label());
Expand Down Expand Up @@ -3291,7 +3291,7 @@ class CheckStackOverflowSlowPath
kStackOverflowRuntimeEntry, kNumSlowPathArgs, locs);
}

if (compiler->isolate()->use_osr() && !compiler->is_optimizing() &&
if (compiler->isolate_group()->use_osr() && !compiler->is_optimizing() &&
instruction()->in_loop()) {
// In unoptimized code, record loop stack checks as possible OSR entries.
compiler->AddCurrentDescriptor(PcDescriptorsLayout::kOsrEntry,
Expand All @@ -3306,7 +3306,7 @@ class CheckStackOverflowSlowPath
}

compiler::Label* osr_entry_label() {
ASSERT(Isolate::Current()->use_osr());
ASSERT(IsolateGroup::Current()->use_osr());
return &osr_entry_label_;
}

Expand Down
6 changes: 3 additions & 3 deletions runtime/vm/compiler/backend/il_ia32.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3025,7 +3025,7 @@ class CheckStackOverflowSlowPath
: TemplateSlowPathCode(instruction) {}

virtual void EmitNativeCode(FlowGraphCompiler* compiler) {
if (compiler->isolate()->use_osr() && osr_entry_label()->IsLinked()) {
if (compiler->isolate_group()->use_osr() && osr_entry_label()->IsLinked()) {
__ Comment("CheckStackOverflowSlowPathOsr");
__ Bind(osr_entry_label());
__ movl(compiler::Address(THR, Thread::stack_overflow_flags_offset()),
Expand All @@ -3044,7 +3044,7 @@ class CheckStackOverflowSlowPath
instruction()->source(), instruction()->deopt_id(),
kStackOverflowRuntimeEntry, 0, instruction()->locs());

if (compiler->isolate()->use_osr() && !compiler->is_optimizing() &&
if (compiler->isolate_group()->use_osr() && !compiler->is_optimizing() &&
instruction()->in_loop()) {
// In unoptimized code, record loop stack checks as possible OSR entries.
compiler->AddCurrentDescriptor(PcDescriptorsLayout::kOsrEntry,
Expand All @@ -3057,7 +3057,7 @@ class CheckStackOverflowSlowPath
}

compiler::Label* osr_entry_label() {
ASSERT(Isolate::Current()->use_osr());
ASSERT(IsolateGroup::Current()->use_osr());
return &osr_entry_label_;
}

Expand Down
6 changes: 3 additions & 3 deletions runtime/vm/compiler/backend/il_x64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3271,7 +3271,7 @@ class CheckStackOverflowSlowPath
: TemplateSlowPathCode(instruction) {}

virtual void EmitNativeCode(FlowGraphCompiler* compiler) {
if (compiler->isolate()->use_osr() && osr_entry_label()->IsLinked()) {
if (compiler->isolate_group()->use_osr() && osr_entry_label()->IsLinked()) {
__ Comment("CheckStackOverflowSlowPathOsr");
__ Bind(osr_entry_label());
__ movq(compiler::Address(THR, Thread::stack_overflow_flags_offset()),
Expand Down Expand Up @@ -3308,7 +3308,7 @@ class CheckStackOverflowSlowPath
kStackOverflowRuntimeEntry, kNumSlowPathArgs, instruction()->locs());
}

if (compiler->isolate()->use_osr() && !compiler->is_optimizing() &&
if (compiler->isolate_group()->use_osr() && !compiler->is_optimizing() &&
instruction()->in_loop()) {
// In unoptimized code, record loop stack checks as possible OSR entries.
compiler->AddCurrentDescriptor(PcDescriptorsLayout::kOsrEntry,
Expand All @@ -3323,7 +3323,7 @@ class CheckStackOverflowSlowPath
}

compiler::Label* osr_entry_label() {
ASSERT(Isolate::Current()->use_osr());
ASSERT(IsolateGroup::Current()->use_osr());
return &osr_entry_label_;
}

Expand Down
2 changes: 1 addition & 1 deletion runtime/vm/compiler/backend/slot.cc
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ const Slot& Slot::Get(const Field& field,
// itself stays behind in the compilation global cache. Thus we must always
// try to add it to the list of guarded fields of the current function.
if (slot.is_guarded_field()) {
if (thread->isolate()->use_field_guards()) {
if (thread->isolate_group()->use_field_guards()) {
ASSERT(parsed_function != nullptr);
parsed_function->AddToGuardedFields(&slot.field());
} else {
Expand Down
4 changes: 2 additions & 2 deletions runtime/vm/compiler/call_specializer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
namespace dart {

// Quick access to the current isolate and zone.
#define I (isolate())
#define IG (isolate_group())
#define Z (zone())

static void RefineUseTypes(Definition* instr) {
Expand Down Expand Up @@ -839,7 +839,7 @@ bool CallSpecializer::TryInlineInstanceSetter(InstanceCallInstr* instr) {
}
}

if (I->use_field_guards()) {
if (IG->use_field_guards()) {
if (field.guarded_cid() != kDynamicCid) {
InsertBefore(instr,
new (Z)
Expand Down
1 change: 1 addition & 0 deletions runtime/vm/compiler/call_specializer.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ class CallSpecializer : public FlowGraphVisitor {
protected:
Thread* thread() const { return flow_graph_->thread(); }
Isolate* isolate() const { return flow_graph_->isolate(); }
IsolateGroup* isolate_group() const { return flow_graph_->isolate_group(); }
Zone* zone() const { return flow_graph_->zone(); }
const Function& function() const { return flow_graph_->function(); }

Expand Down
8 changes: 4 additions & 4 deletions runtime/vm/compiler/frontend/base_flow_graph_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace dart {
namespace kernel {

#define Z (zone_)
#define I (thread_->isolate())
#define IG (thread_->isolate_group())

Fragment& Fragment::operator+=(const Fragment& other) {
if (entry == NULL) {
Expand Down Expand Up @@ -546,7 +546,7 @@ Fragment BaseFlowGraphBuilder::StoreInstanceFieldGuarded(
kind /* = StoreInstanceFieldInstr::Kind::kOther */) {
Fragment instructions;
const Field& field_clone = MayCloneField(Z, field);
if (I->use_field_guards()) {
if (IG->use_field_guards()) {
LocalVariable* store_expression = MakeTemporary();
instructions += LoadLocal(store_expression);
instructions += GuardFieldClass(field_clone, GetNextDeoptId());
Expand Down Expand Up @@ -874,7 +874,7 @@ JoinEntryInstr* BaseFlowGraphBuilder::BuildThrowNoSuchMethod() {

Fragment failing(nsm);
const Code& nsm_handler = Code::ZoneHandle(
Z, I->object_store()->call_closure_no_such_method_stub());
Z, IG->object_store()->call_closure_no_such_method_stub());
failing += LoadArgDescriptor();
failing += TailCall(nsm_handler);

Expand Down Expand Up @@ -906,7 +906,7 @@ Fragment BaseFlowGraphBuilder::AllocateContext(
Fragment BaseFlowGraphBuilder::AllocateClosure(
TokenPosition position,
const Function& closure_function) {
const Class& cls = Class::ZoneHandle(Z, I->object_store()->closure_class());
const Class& cls = Class::ZoneHandle(Z, IG->object_store()->closure_class());
AllocateObjectInstr* allocate =
new (Z) AllocateObjectInstr(InstructionSource(position), cls);
allocate->set_closure_function(closure_function);
Expand Down
5 changes: 3 additions & 2 deletions runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ namespace kernel {
#define H (translation_helper_)
#define T (type_translator_)
#define I Isolate::Current()
#define IG IsolateGroup::Current()
#define B (flow_graph_builder_)

Class& StreamingFlowGraphBuilder::GetSuperOrDie() {
Expand Down Expand Up @@ -3920,7 +3921,7 @@ Fragment StreamingFlowGraphBuilder::BuildEmptyStatement() {
}

Fragment StreamingFlowGraphBuilder::BuildAssertBlock() {
if (!I->asserts()) {
if (!IG->asserts()) {
SkipStatementList();
return Fragment();
}
Expand All @@ -3944,7 +3945,7 @@ Fragment StreamingFlowGraphBuilder::BuildAssertBlock() {
}

Fragment StreamingFlowGraphBuilder::BuildAssertStatement() {
if (!I->asserts()) {
if (!IG->asserts()) {
SetOffset(ReaderOffset() - 1); // Include the tag.
SkipStatement(); // read this statement.
return Fragment();
Expand Down
2 changes: 1 addition & 1 deletion runtime/vm/compiler/frontend/kernel_to_il.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4464,7 +4464,7 @@ Fragment FlowGraphBuilder::NullAssertion(LocalVariable* variable) {

Fragment FlowGraphBuilder::BuildNullAssertions() {
Fragment code;
if (IG->null_safety() || !I->asserts() || !FLAG_null_assertions) {
if (IG->null_safety() || !IG->asserts() || !FLAG_null_assertions) {
return code;
}

Expand Down
5 changes: 3 additions & 2 deletions runtime/vm/compiler/frontend/scope_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ namespace kernel {
#define H (translation_helper_)
#define T (type_translator_)
#define I Isolate::Current()
#define IG IsolateGroup::Current()

ScopeBuilder::ScopeBuilder(ParsedFunction* parsed_function)
: result_(NULL),
Expand Down Expand Up @@ -964,7 +965,7 @@ void ScopeBuilder::VisitStatement() {
case kEmptyStatement:
return;
case kAssertBlock:
if (I->asserts()) {
if (IG->asserts()) {
PositionScope scope(&helper_.reader_);
intptr_t offset =
helper_.ReaderOffset() - 1; // -1 to include tag byte.
Expand All @@ -984,7 +985,7 @@ void ScopeBuilder::VisitStatement() {
}
return;
case kAssertStatement:
if (I->asserts()) {
if (IG->asserts()) {
VisitExpression(); // Read condition.
helper_.ReadPosition(); // read condition start offset.
helper_.ReadPosition(); // read condition end offset.
Expand Down
18 changes: 10 additions & 8 deletions runtime/vm/dart.cc
Original file line number Diff line number Diff line change
Expand Up @@ -981,6 +981,8 @@ ErrorPtr Dart::InitializeIsolate(const uint8_t* snapshot_data,
const char* Dart::FeaturesString(Isolate* isolate,
bool is_vm_isolate,
Snapshot::Kind kind) {
auto isolate_group = isolate != nullptr ? isolate->group() : nullptr;

TextBuffer buffer(64);

// Different fields are included for DEBUG/RELEASE/PRODUCT.
Expand All @@ -1001,21 +1003,22 @@ const char* Dart::FeaturesString(Isolate* isolate,
#define ADD_C(name, PCV, PV, T, DV, C) ADD_FLAG(name, FLAG_##name)
#define ADD_D(name, T, DV, C) ADD_FLAG(name, FLAG_##name)

#define ADD_ISOLATE_FLAG(name, isolate_flag, flag) \
#define ADD_ISOLATE_GROUP_FLAG(name, isolate_flag, flag) \
do { \
const bool value = (isolate != NULL) ? isolate->name() : flag; \
const bool value = \
isolate_group != nullptr ? isolate_group->name() : flag; \
ADD_FLAG(#name, value); \
} while (0);

if (Snapshot::IncludesCode(kind)) {
VM_GLOBAL_FLAG_LIST(ADD_P, ADD_R, ADD_C, ADD_D);

// enabling assertions affects deopt ids.
ADD_ISOLATE_FLAG(asserts, enable_asserts, FLAG_enable_asserts);
// Enabling assertions affects deopt ids.
ADD_ISOLATE_GROUP_FLAG(asserts, enable_asserts, FLAG_enable_asserts);
if (kind == Snapshot::kFullJIT) {
ADD_ISOLATE_FLAG(use_field_guards, use_field_guards,
FLAG_use_field_guards);
ADD_ISOLATE_FLAG(use_osr, use_osr, FLAG_use_osr);
ADD_ISOLATE_GROUP_FLAG(use_field_guards, use_field_guards,
FLAG_use_field_guards);
ADD_ISOLATE_GROUP_FLAG(use_osr, use_osr, FLAG_use_osr);
}
#if !defined(PRODUCT)
buffer.AddString(FLAG_code_comments ? " code-comments"
Expand Down Expand Up @@ -1053,7 +1056,6 @@ const char* Dart::FeaturesString(Isolate* isolate,
}

if (!Snapshot::IsAgnosticToNullSafety(kind)) {
auto isolate_group = isolate != nullptr ? isolate->group() : nullptr;
if (isolate_group != nullptr) {
if (isolate_group->null_safety()) {
buffer.AddString(" null-safety");
Expand Down
Loading

0 comments on commit 897b1a2

Please sign in to comment.