dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Reland "[vm] Cleanup old async/async*/sync* implementation from the VM"

Browse Source 
This is a reland of commit bc8afad855

On top of the original commit, this change fixes incorrect
propagation of async/async*/sync* modifiers from a function to
its dynamic invocation forwarder.

TEST=ci, runtime/tests/vm/dart/regress_b_238653741_test.dart
Fixes b/238653741
Issue: https://github.com/dart-lang/sdk/issues/48378

Original change's description:
> [vm] Cleanup old async/async*/sync* implementation from the VM
>
> TEST=ci
>
> Issue: https://github.com/dart-lang/sdk/issues/48378
> Change-Id: I089ba4ed5613f30eec29f0db4ac6d5d8fbffd185
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/249980
> Reviewed-by: Martin Kustermann <kustermann@google.com>
> Reviewed-by: Slava Egorov <vegorov@google.com>
> Commit-Queue: Alexander Markov <alexmarkov@google.com>

Change-Id: Iaad033d974a23fc6c5880a3d7f41818eb117f839
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/251300
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
This commit is contained in:
Alexander Markov
2022-07-12 19:25:32 +00:00
committed by Commit Bot
parent 91a9774010
commit 61caeca47a
28 changed files with 517 additions and 1614 deletions
Download Patch File Download Diff File Expand all files Collapse all files
runtime/vm/compiler/aot/precompiler.cc
-17
View File
@@ -629,8 +629,6 @@ void Precompiler::DoCompileAll() {
IG->object_store()->set_simple_instance_of_true_function(null_function);
IG->object_store()->set_simple_instance_of_false_function(
null_function);
IG->object_store()->set_async_star_move_next_helper(null_function);
IG->object_store()->set_complete_on_async_return(null_function);
IG->object_store()->set_async_star_stream_controller(null_class);
DropMetadata();
DropLibraryEntries();
@@ -1136,14 +1134,6 @@ void Precompiler::AddTypesOf(const Function& function) {
return;
}
// Preserve parents for generated bodies in async/async*/sync* functions,
// since predicates like Function::IsAsyncClosure(), etc. need that info.
if (function.is_generated_body()) {
AddRetainReason(parent_function, RetainReasons::kIsSyncAsyncFunction);
AddTypesOf(parent_function);
return;
}
// We're not retaining the parent due to this function, so wrap it with
// a weak serialization reference.
const auto& data = ClosureData::CheckedHandle(Z, function.data());
@@ -2983,10 +2973,6 @@ void Precompiler::DiscardCodeObjects() {
++codes_with_native_function_;
return;
}
if (function_.IsAsyncClosure() || function_.IsAsyncGenClosure()) {
++codes_with_async_closure_function_;
return;
}
// Retain Code objects corresponding to dynamically
// called functions.
@@ -3040,8 +3026,6 @@ void Precompiler::DiscardCodeObjects() {
codes_with_pc_descriptors_);
THR_Print(" %8" Pd " Codes with native functions\n",
codes_with_native_function_);
THR_Print(" %8" Pd " Codes with async closure functions\n",
codes_with_async_closure_function_);
THR_Print(" %8" Pd " Codes with dynamically called functions\n",
codes_with_dynamically_called_function_);
THR_Print(" %8" Pd " Codes with deferred functions\n",
@@ -3073,7 +3057,6 @@ void Precompiler::DiscardCodeObjects() {
intptr_t codes_with_exception_handlers_ = 0;
intptr_t codes_with_pc_descriptors_ = 0;
intptr_t codes_with_native_function_ = 0;
intptr_t codes_with_async_closure_function_ = 0;
intptr_t codes_with_dynamically_called_function_ = 0;
intptr_t codes_with_deferred_function_ = 0;
intptr_t codes_with_ffi_trampoline_function_ = 0;
runtime/vm/compiler/backend/il.cc
+3 -3
View File
@@ -6837,7 +6837,7 @@ void RawStoreFieldInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
const Code& ReturnInstr::GetReturnStub(FlowGraphCompiler* compiler) const {
const Function& function = compiler->parsed_function().function();
ASSERT(function.IsSuspendableFunction());
if (function.IsCompactAsyncFunction()) {
if (function.IsAsyncFunction()) {
if (!value()->Type()->CanBeFuture()) {
return Code::ZoneHandle(compiler->zone(),
compiler->isolate_group()
@@ -6847,11 +6847,11 @@ const Code& ReturnInstr::GetReturnStub(FlowGraphCompiler* compiler) const {
return Code::ZoneHandle(
compiler->zone(),
compiler->isolate_group()->object_store()->return_async_stub());
} else if (function.IsCompactAsyncStarFunction()) {
} else if (function.IsAsyncGenerator()) {
return Code::ZoneHandle(
compiler->zone(),
compiler->isolate_group()->object_store()->return_async_star_stub());
} else if (function.IsCompactSyncStarFunction()) {
} else if (function.IsSyncGenerator()) {
return Code::ZoneHandle(
compiler->zone(),
compiler->isolate_group()->object_store()->return_sync_star_stub());
runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc
+51 -221
View File
@@ -482,11 +482,6 @@ Fragment StreamingFlowGraphBuilder::TypeArgumentsHandling(
if (dart_function.IsClosureFunction() &&
dart_function.NumParentTypeArguments() > 0) {
LocalVariable* closure = parsed_function()->ParameterVariable(0);
// Function with yield points can not be generic itself but the outer
// function can be.
ASSERT(yield_continuations().is_empty() || !dart_function.IsGeneric());
LocalVariable* fn_type_args = parsed_function()->function_type_arguments();
ASSERT(fn_type_args != nullptr && closure != nullptr);
@@ -518,111 +513,6 @@ Fragment StreamingFlowGraphBuilder::TypeArgumentsHandling(
return prologue;
}
Fragment StreamingFlowGraphBuilder::CompleteBodyWithYieldContinuations(
Fragment body) {
// The code we are building will be executed right after we enter
// the function and before any nested contexts are allocated.
// Reset current context_depth_ to match this.
const intptr_t current_context_depth = B->context_depth_;
B->context_depth_ = scopes()->yield_jump_variable->owner()->context_level();
// Prepend an entry corresponding to normal entry to the function.
yield_continuations().InsertAt(
0,
YieldContinuation(new (Z) DropTempsInstr(0, nullptr), kInvalidTryIndex));
yield_continuations()[0].entry->LinkTo(body.entry);
// Load :await_jump_var into a temporary.
Fragment dispatch;
dispatch += LoadLocal(scopes()->yield_jump_variable);
dispatch += StoreLocal(TokenPosition::kNoSource, scopes()->switch_variable);
dispatch += Drop();
const intptr_t continuation_count = yield_continuations().length();
IndirectGotoInstr* indirect_goto;
if (FLAG_async_igoto_threshold >= 0 &&
continuation_count >= FLAG_async_igoto_threshold) {
dispatch += LoadLocal(scopes()->switch_variable);
dispatch += IndirectGoto(continuation_count);
indirect_goto = dispatch.current->AsIndirectGoto();
for (intptr_t i = 0; i < continuation_count; i++) {
if (i >= 1) {
Fragment resumption;
// Every continuation after the first is not a normal entry but a
// resumption.
// Restore :current_context_var from :await_ctx_var.
// Note: after this point context_depth_ does not match current context
// depth so we should not access any local variables anymore.
resumption += LoadLocal(scopes()->yield_context_variable);
resumption += StoreLocal(TokenPosition::kNoSource,
parsed_function()->current_context_var());
resumption += Drop();
Instruction* next = yield_continuations()[i].entry->next();
yield_continuations()[i].entry->LinkTo(resumption.entry);
resumption <<= next;
}
IndirectEntryInstr* indirect_entry = B->BuildIndirectEntry(
/*indirect_id=*/i, yield_continuations()[i].try_index);
indirect_entry->LinkTo(yield_continuations()[i].entry->next());
TargetEntryInstr* target = B->BuildTargetEntry();
Fragment(target) + Goto(indirect_entry);
indirect_goto->AddSuccessor(target);
}
} else {
BlockEntryInstr* block = nullptr;
for (intptr_t i = 0; i < continuation_count; i++) {
if (i == 1) {
// This is not a normal entry but a resumption. Restore
// :current_context_var from :await_ctx_var.
// Note: after this point context_depth_ does not match current context
// depth so we should not access any local variables anymore.
dispatch += LoadLocal(scopes()->yield_context_variable);
dispatch += StoreLocal(TokenPosition::kNoSource,
parsed_function()->current_context_var());
dispatch += Drop();
}
if (i == (continuation_count - 1)) {
// We reached the last possibility, no need to build more ifs.
// Continue to the last continuation.
// Note: continuations start with nop DropTemps instruction
// which acts like an anchor, so we need to skip it.
block->set_try_index(yield_continuations()[i].try_index);
dispatch <<= yield_continuations()[i].entry->next();
break;
}
// Build comparison:
//
// if (:await_jump_var == i) {
// -> yield_continuations()[i]
// } else ...
//
TargetEntryInstr* then;
TargetEntryInstr* otherwise;
dispatch += LoadLocal(scopes()->switch_variable);
dispatch += IntConstant(i);
dispatch += B->BranchIfStrictEqual(&then, &otherwise);
// True branch is linked to appropriate continuation point.
// Note: continuations start with nop DropTemps instruction
// which acts like an anchor, so we need to skip it.
then->LinkTo(yield_continuations()[i].entry->next());
then->set_try_index(yield_continuations()[i].try_index);
// False branch will contain the next comparison.
dispatch = Fragment(dispatch.entry, otherwise);
block = otherwise;
}
}
B->context_depth_ = current_context_depth;
return dispatch;
}
Fragment StreamingFlowGraphBuilder::CheckStackOverflowInPrologue(
const Function& dart_function) {
if (dart_function.is_native()) return {};
@@ -669,7 +559,7 @@ Fragment StreamingFlowGraphBuilder::SetupCapturedParameters(
Fragment StreamingFlowGraphBuilder::InitSuspendableFunction(
const Function& dart_function) {
Fragment body;
if (dart_function.IsCompactAsyncFunction()) {
if (dart_function.IsAsyncFunction()) {
const auto& result_type =
AbstractType::Handle(Z, dart_function.result_type());
auto& type_args = TypeArguments::ZoneHandle(Z);
@@ -684,7 +574,7 @@ Fragment StreamingFlowGraphBuilder::InitSuspendableFunction(
body += B->Call1ArgStub(TokenPosition::kNoSource,
Call1ArgStubInstr::StubId::kInitAsync);
body += Drop();
} else if (dart_function.IsCompactAsyncStarFunction()) {
} else if (dart_function.IsAsyncGenerator()) {
const auto& result_type =
AbstractType::Handle(Z, dart_function.result_type());
auto& type_args = TypeArguments::ZoneHandle(Z);
@@ -702,7 +592,7 @@ Fragment StreamingFlowGraphBuilder::InitSuspendableFunction(
body += B->Suspend(TokenPosition::kNoSource,
SuspendInstr::StubId::kYieldAsyncStar);
body += Drop();
} else if (dart_function.IsCompactSyncStarFunction()) {
} else if (dart_function.IsSyncGenerator()) {
const auto& result_type =
AbstractType::Handle(Z, dart_function.result_type());
auto& type_args = TypeArguments::ZoneHandle(Z);
@@ -957,63 +847,46 @@ FlowGraph* StreamingFlowGraphBuilder::BuildGraphOfFunction(
every_time_prologue, type_args_handling);
Fragment function(instruction_cursor);
if (yield_continuations().is_empty()) {
FunctionEntryInstr* extra_entry = nullptr;
switch (extra_entry_point_style) {
case UncheckedEntryPointStyle::kNone: {
function += every_time_prologue + first_time_prologue +
type_args_handling + implicit_type_checks +
explicit_type_checks + body;
break;
}
case UncheckedEntryPointStyle::kSeparate: {
ASSERT(instruction_cursor == normal_entry);
ASSERT(first_time_prologue.is_empty());
ASSERT(type_args_handling.is_empty());
const Fragment prologue_copy = BuildEveryTimePrologue(
dart_function, token_position, type_parameters_offset);
extra_entry = B->BuildSeparateUncheckedEntryPoint(
normal_entry,
/*normal_prologue=*/every_time_prologue + implicit_type_checks,
/*extra_prologue=*/prologue_copy,
/*shared_prologue=*/explicit_type_checks,
/*body=*/body);
break;
}
case UncheckedEntryPointStyle::kSharedWithVariable: {
Fragment prologue(normal_entry, instruction_cursor);
prologue += every_time_prologue;
prologue += first_time_prologue;
prologue += type_args_handling;
prologue += explicit_type_checks;
extra_entry = B->BuildSharedUncheckedEntryPoint(
/*shared_prologue_linked_in=*/prologue,
/*skippable_checks=*/implicit_type_checks,
/*redefinitions_if_skipped=*/implicit_redefinitions,
/*body=*/body);
break;
}
FunctionEntryInstr* extra_entry = nullptr;
switch (extra_entry_point_style) {
case UncheckedEntryPointStyle::kNone: {
function += every_time_prologue + first_time_prologue +
type_args_handling + implicit_type_checks +
explicit_type_checks + body;
break;
}
if (extra_entry != nullptr) {
B->RecordUncheckedEntryPoint(graph_entry, extra_entry);
case UncheckedEntryPointStyle::kSeparate: {
ASSERT(instruction_cursor == normal_entry);
ASSERT(first_time_prologue.is_empty());
ASSERT(type_args_handling.is_empty());
const Fragment prologue_copy = BuildEveryTimePrologue(
dart_function, token_position, type_parameters_offset);
extra_entry = B->BuildSeparateUncheckedEntryPoint(
normal_entry,
/*normal_prologue=*/every_time_prologue + implicit_type_checks,
/*extra_prologue=*/prologue_copy,
/*shared_prologue=*/explicit_type_checks,
/*body=*/body);
break;
}
} else {
// If the function's body contains any yield points, build switch statement
// that selects a continuation point based on the value of :await_jump_var.
ASSERT(explicit_type_checks.is_empty());
// If the function is generic, type_args_handling might require access to
// (possibly captured) 'this' for preparing default type arguments, in which
// case we can't run it before the 'first_time_prologue'.
ASSERT(!dart_function.IsGeneric());
// TODO(#34162): We can probably ignore the implicit checks
// here as well since the arguments are passed from generated code.
function += every_time_prologue + type_args_handling +
CompleteBodyWithYieldContinuations(first_time_prologue +
implicit_type_checks + body);
case UncheckedEntryPointStyle::kSharedWithVariable: {
Fragment prologue(normal_entry, instruction_cursor);
prologue += every_time_prologue;
prologue += first_time_prologue;
prologue += type_args_handling;
prologue += explicit_type_checks;
extra_entry = B->BuildSharedUncheckedEntryPoint(
/*shared_prologue_linked_in=*/prologue,
/*skippable_checks=*/implicit_type_checks,
/*redefinitions_if_skipped=*/implicit_redefinitions,
/*body=*/body);
break;
}
}
if (extra_entry != nullptr) {
B->RecordUncheckedEntryPoint(graph_entry, extra_entry);
}
// When compiling for OSR, use a depth first search to find the OSR
@@ -1517,11 +1390,6 @@ BreakableBlock* StreamingFlowGraphBuilder::breakable_block() {
return flow_graph_builder_->breakable_block_;
}
GrowableArray<YieldContinuation>&
StreamingFlowGraphBuilder::yield_continuations() {
return flow_graph_builder_->yield_continuations_;
}
Value* StreamingFlowGraphBuilder::stack() {
return flow_graph_builder_->stack_;
}
@@ -4390,8 +4258,8 @@ Fragment StreamingFlowGraphBuilder::BuildLibraryPrefixAction(
Fragment StreamingFlowGraphBuilder::BuildAwaitExpression(
TokenPosition* position) {
ASSERT(parsed_function()->function().IsCompactAsyncFunction() ||
parsed_function()->function().IsCompactAsyncStarFunction());
ASSERT(parsed_function()->function().IsAsyncFunction() ||
parsed_function()->function().IsAsyncGenerator());
Fragment instructions;
const TokenPosition pos = ReadPosition(); // read file offset.
@@ -5323,7 +5191,7 @@ Fragment StreamingFlowGraphBuilder::BuildYieldStatement(
instructions += DebugStepCheck(pos);
}
if (parsed_function()->function().IsCompactAsyncStarFunction()) {
if (parsed_function()->function().IsAsyncGenerator()) {
// In the async* functions, generate the following code for yield <expr>:
//
// _AsyncStarStreamController controller = :suspend_state._functionData;
@@ -5377,7 +5245,7 @@ Fragment StreamingFlowGraphBuilder::BuildYieldStatement(
instructions += Drop();
}
} else if (parsed_function()->function().IsCompactSyncStarFunction()) {
} else if (parsed_function()->function().IsSyncGenerator()) {
// In the sync* functions, generate the following code for yield <expr>:
//
// _SyncStarIterator iterator = :suspend_state._functionData;
@@ -5568,62 +5436,24 @@ Fragment StreamingFlowGraphBuilder::BuildFunctionNode(
if (function_node_helper.async_marker_ == FunctionNodeHelper::kAsync) {
function.set_modifier(UntaggedFunction::kAsync);
function.set_is_debuggable(true);
function.set_is_inlinable(false);
function.set_is_visible(true);
ASSERT(function.IsCompactAsyncFunction());
ASSERT(function.IsAsyncFunction());
} else if (function_node_helper.async_marker_ ==
FunctionNodeHelper::kAsyncStar) {
function.set_modifier(UntaggedFunction::kAsyncGen);
function.set_is_debuggable(true);
function.set_is_inlinable(false);
function.set_is_visible(true);
ASSERT(function.IsCompactAsyncStarFunction());
ASSERT(function.IsAsyncGenerator());
} else if (function_node_helper.async_marker_ ==
FunctionNodeHelper::kSyncStar) {
function.set_modifier(UntaggedFunction::kSyncGen);
function.set_is_debuggable(true);
function.set_is_inlinable(false);
function.set_is_visible(true);
ASSERT(function.IsCompactSyncStarFunction());
ASSERT(function.IsSyncGenerator());
} else {
ASSERT(function_node_helper.async_marker_ ==
FunctionNodeHelper::kSync);
function.set_is_debuggable(function_node_helper.dart_async_marker_ ==
FunctionNodeHelper::kSync);
switch (function_node_helper.dart_async_marker_) {
case FunctionNodeHelper::kSyncStar:
function.set_modifier(UntaggedFunction::kSyncGen);
break;
case FunctionNodeHelper::kAsync:
function.set_modifier(UntaggedFunction::kAsync);
break;
case FunctionNodeHelper::kAsyncStar:
function.set_modifier(UntaggedFunction::kAsyncGen);
break;
default:
// no special modifier
break;
}
function.set_is_generated_body(false);
// sync* functions contain two nested synthetic functions,
// the first of which (sync_op_gen) is a regular sync function so we
// need to manually label it generated:
if (function.parent_function() != Function::null()) {
const auto& parent = Function::Handle(function.parent_function());
if (parent.IsSyncGenerator() &&
!parent.IsCompactSyncStarFunction()) {
function.set_is_generated_body(true);
}
}
// Note: Is..() methods use the modifiers set above, so order
// matters.
if (function.IsAsyncClosure() || function.IsAsyncGenClosure()) {
function.set_is_inlinable(false);
}
ASSERT(!function.IsCompactAsyncFunction());
ASSERT(!function.IsCompactAsyncStarFunction());
ASSERT(!function.IsCompactSyncStarFunction());
ASSERT(!function.IsAsyncFunction());
ASSERT(!function.IsAsyncGenerator());
ASSERT(!function.IsSyncGenerator());
}
// If the start token position is synthetic, the end token position
runtime/vm/compiler/frontend/kernel_binary_flowgraph.h
-2
View File
@@ -98,7 +98,6 @@ class StreamingFlowGraphBuilder : public KernelReaderHelper {
Fragment ShortcutForUserDefinedEquals(const Function& dart_function,
LocalVariable* first_parameter);
Fragment TypeArgumentsHandling(const Function& dart_function);
Fragment CompleteBodyWithYieldContinuations(Fragment body);
static UncheckedEntryPointStyle ChooseEntryPointStyle(
const Function& dart_function,
@@ -131,7 +130,6 @@ class StreamingFlowGraphBuilder : public KernelReaderHelper {
TryFinallyBlock* try_finally_block();
SwitchBlock* switch_block();
BreakableBlock* breakable_block();
GrowableArray<YieldContinuation>& yield_continuations();
Value* stack();
void set_stack(Value* top);
void Push(Definition* definition);
runtime/vm/compiler/frontend/kernel_to_il.h
-12
View File
@@ -35,16 +35,6 @@ class SwitchBlock;
class TryCatchBlock;
class TryFinallyBlock;
struct YieldContinuation {
Instruction* entry;
intptr_t try_index;
YieldContinuation(Instruction* entry, intptr_t try_index)
: entry(entry), try_index(try_index) {}
YieldContinuation() : entry(NULL), try_index(kInvalidTryIndex) {}
};
enum class TypeChecksToBuild {
kCheckAllTypeParameterBounds,
kCheckNonCovariantTypeParameterBounds,
@@ -604,8 +594,6 @@ class FlowGraphBuilder : public BaseFlowGraphBuilder {
ScopeBuildingResult* scopes_;
GrowableArray<YieldContinuation> yield_continuations_;
LocalVariable* CurrentException() {
return scopes_->exception_variables[catch_depth_ - 1];
}
runtime/vm/compiler/frontend/scope_builder.cc
+2 -22
View File
@@ -206,17 +206,7 @@ ScopeBuildingResult* ScopeBuilder::BuildScopes() {
ParameterTypeCheckMode type_check_mode =
kTypeCheckForNonDynamicallyInvokedMethod;
if (function.IsSyncGenClosure()) {
// Don't type check the parameter of sync-yielding since these calls are
// all synthetic and types should always match.
ASSERT_EQUAL(
function.NumParameters() - function.NumImplicitParameters(), 3);
ASSERT(
Class::Handle(
AbstractType::Handle(function.ParameterTypeAt(1)).type_class())
.ScrubbedName() == Symbols::_SyncIterator().ptr());
type_check_mode = kTypeCheckForStaticFunction;
} else if (function.is_static()) {
if (function.is_static()) {
// In static functions we don't check anything.
type_check_mode = kTypeCheckForStaticFunction;
} else if (function.IsImplicitClosureFunction()) {
@@ -1325,17 +1315,7 @@ void ScopeBuilder::VisitVariableDeclaration() {
variable->set_late_init_offset(initializer_offset);
}
// Lift the special async vars out of the function body scope, into the
// outer function declaration scope.
// This way we can allocate them in the outermost context at fixed indices,
// allowing support for async stack traces implementation to find awaiters.
if (name.Equals(Symbols::AwaitJumpVar()) ||
name.Equals(Symbols::AsyncFuture()) || name.Equals(Symbols::is_sync()) ||
name.Equals(Symbols::Controller())) {
scope_->parent()->AddVariable(variable);
} else {
scope_->AddVariable(variable);
}
scope_->AddVariable(variable);
result_->locals.Insert(helper_.data_program_offset_ + kernel_offset_no_tag,
variable);
}
runtime/vm/compiler/recognized_methods_list.h
-1
View File
@@ -280,7 +280,6 @@ namespace dart {
V(::, _asExternalTypedDataDouble, FfiAsExternalTypedDataDouble, 0x40cdd9e1) \
V(::, _getNativeField, GetNativeField, 0xa0139b85) \
V(::, reachabilityFence, ReachabilityFence, 0x730f2b7f) \
V(::, _asyncThenWrapperHelper, AsyncThenWrapperHelper, 0x0c17f838) \
V(_Utf8Decoder, _scan, Utf8DecoderScan, 0xf296c901) \
V(_Future, timeout, FutureTimeout, 0xa7cb3294) \
V(Future, wait, FutureWait, 0xb0b596bd) \
runtime/vm/compiler/runtime_offsets_extracted.h
+330 -330
View File
File diff suppressed because it is too large Load Diff