Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(862)

Unified Diff: runtime/vm/kernel_binary_flowgraph.cc

Issue 2891053003: Add support for converted closures with explicit contexts to VM (Closed)
Patch Set: Return statement-to-block conversion for procedure bodies Created 3 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: runtime/vm/kernel_binary_flowgraph.cc
diff --git a/runtime/vm/kernel_binary_flowgraph.cc b/runtime/vm/kernel_binary_flowgraph.cc
index 37f596c4fa2535ec5c15ee3e6198bac1a1ef6388..5db62d49d71b54397e2b5c0955947daec5fcd9ac 100644
--- a/runtime/vm/kernel_binary_flowgraph.cc
+++ b/runtime/vm/kernel_binary_flowgraph.cc
@@ -121,6 +121,7 @@ ScopeBuildingResult* StreamingScopeBuilder::BuildScopes() {
switch (function.kind()) {
case RawFunction::kClosureFunction:
+ case RawFunction::kConvertedClosureFunction:
case RawFunction::kRegularFunction:
case RawFunction::kGetterFunction:
case RawFunction::kSetterFunction:
@@ -359,10 +360,10 @@ void StreamingScopeBuilder::VisitProcedure() {
void StreamingScopeBuilder::VisitField() {
builder_->ReadFieldUntilAnnotation(
&unused_nameindex, &unused_tokenposition, &unused_tokenposition,
- &unused_intptr, &unused_word); // read first part of field.
- builder_->SkipListOfExpressions(); // read annotations.
- VisitDartType(); // read type.
- Tag tag = builder_->ReadTag(); // read initializer (part 1).
+ &unused_intptr, &unused_word); // read first part of field.
+ builder_->SkipListOfExpressions(); // read annotations.
+ VisitDartType(); // read type.
+ Tag tag = builder_->ReadTag(); // read initializer (part 1).
if (tag == kSomething) {
VisitExpression(); // read initializer (part 2).
}
@@ -693,6 +694,26 @@ void StreamingScopeBuilder::VisitExpression() {
return;
case kNullLiteral:
return;
+ case kVectorCreation:
+ builder_->ReadUInt(); // read size.
+ return;
+ case kVectorGet:
+ VisitExpression(); // read expression.
+ builder_->ReadUInt(); // read index.
+ return;
+ case kVectorSet:
+ VisitExpression(); // read vector expression.
+ builder_->ReadUInt(); // read index.
+ VisitExpression(); // read value.
+ return;
+ case kVectorCopy:
+ VisitExpression(); // read vector expression.
+ return;
+ case kClosureCreation:
+ builder_->SkipCanonicalNameReference(); // read function reference.
+ VisitExpression(); // read context vector.
+ VisitDartType(); // read function type of the closure.
+ return;
default:
UNREACHABLE();
}
@@ -1032,6 +1053,7 @@ void StreamingScopeBuilder::VisitDartType() {
case kDynamicType:
case kVoidType:
case kBottomType:
+ case kVectorType:
// those contain nothing.
return;
case kInterfaceType:
@@ -1443,6 +1465,9 @@ void StreamingDartTypeTranslator::BuildTypeInternal() {
case kVoidType:
result_ = Object::void_type().raw();
break;
+ case kVectorType:
+ result_ = Object::vector_type().raw();
+ break;
case kBottomType:
result_ = dart::Class::Handle(Z, I->object_store()->null_class())
.CanonicalType();
@@ -3141,6 +3166,83 @@ FlowGraph* StreamingFlowGraphBuilder::BuildGraphOfImplicitClosureFunction(
flow_graph_builder_->next_block_id_ - 1);
}
+
+// This method follows the logic similar to that of
kustermann 2017/06/20 12:13:14 s/follows the logic .... of/follows the logic of/
Dmitry Stefantsov 2017/06/22 14:12:52 Agreed. It reads better. Thanks!
+// StreamingFlowGraphBuilder::BuildGraphOfImplicitClosureFunction. For
+// additional details on converted closure functions, please, see the comment on
+// the method Function::ConvertedClosureFunction.
+FlowGraph* StreamingFlowGraphBuilder::BuildGraphOfConvertedClosureFunction(
+ const Function& function) {
kustermann 2017/06/20 12:13:13 Given the significant duplication of code, maybe c
Dmitry Stefantsov 2017/06/22 14:12:52 I agree, we may benefit from the parametrization.
kustermann 2017/06/26 10:32:36 Just keep it then.
+ const Function& target = Function::ZoneHandle(Z, function.parent_function());
+
+ TargetEntryInstr* normal_entry = flow_graph_builder_->BuildTargetEntry();
+ flow_graph_builder_->graph_entry_ = new (Z) GraphEntryInstr(
+ *parsed_function(), normal_entry, Compiler::kNoOSRDeoptId);
+ SetupDefaultParameterValues();
+
+ Fragment body(normal_entry);
+ body += flow_graph_builder_->CheckStackOverflowInPrologue();
+
+ // Load all the arguments.
+ ASSERT(target.is_static());
+
+ TokenPosition end_position;
+ ReadFunctionNodeUntilTypeParameters(
+ &unused_tokenposition, &end_position, &unused_word,
+ &unused_word); // read first part of function node.
+ SkipTypeParametersList(); // read type parameter list.
+ ReadUInt(); // read total parameter count.
+ ReadUInt(); // read required_parameter_count.
+
+ // Positional.
+ intptr_t positional_argument_count = ReadListLength();
kustermann 2017/06/20 12:13:14 Usually we try to use "const" where possible. (als
Dmitry Stefantsov 2017/06/22 14:12:52 Do you mean something like `const intptr_t positio
kustermann 2017/06/26 10:32:36 It's fine to just change your code. The benefit of
+
+ // The first argument is the instance of the closure class. For converted
+ // closures its context field contains the context vector that is used by the
+ // converted top-level function (target) explicitly and that should be passed
+ // to that function as the first parameter.
+ body += LoadLocal(LookupVariable(ReaderOffset())); // 0th variable offset.
+ body += flow_graph_builder_->LoadField(Closure::context_offset());
+ body += PushArgument();
+ SkipVariableDeclaration(); // read 0th variable.
+
+ // The rest of the parameters are the same for the method of the Closure class
+ // being invoked and the top-level function (target).
+ for (intptr_t i = 1; i < positional_argument_count; i++) {
+ body += LoadLocal(LookupVariable(ReaderOffset())); // ith variable offset.
+ body += PushArgument();
+ SkipVariableDeclaration(); // read ith variable.
+ }
+
+ // Named.
+ intptr_t named_argument_count = ReadListLength();
+ Array& argument_names = Array::ZoneHandle(Z);
+ if (named_argument_count > 0) {
+ argument_names = Array::New(named_argument_count);
+ for (intptr_t i = 0; i < named_argument_count; i++) {
+ body +=
+ LoadLocal(LookupVariable(ReaderOffset())); // ith variable offset.
+ body += PushArgument();
+ argument_names.SetAt(
+ i, H.DartSymbol(GetNameFromVariableDeclaration(ReaderOffset())));
+ SkipVariableDeclaration(); // read ith variable.
+ }
+ }
+
+ // Forward them to the target.
+ intptr_t argument_count = positional_argument_count + named_argument_count;
+ body += StaticCall(TokenPosition::kNoSource, target, argument_count,
+ argument_names);
+
+ // Return the result.
+ body += Return(end_position);
+
+ return new (Z)
+ FlowGraph(*parsed_function(), flow_graph_builder_->graph_entry_,
+ flow_graph_builder_->next_block_id_ - 1);
+}
+
+
static bool IsGetMainClosure(const String& name) {
if (name.Length() < 16) return false;
const char* cstr = "_getMainClosure@";
@@ -3564,13 +3666,17 @@ FlowGraph* StreamingFlowGraphBuilder::BuildGraph(intptr_t kernel_offset) {
switch (function.kind()) {
case RawFunction::kClosureFunction:
+ case RawFunction::kConvertedClosureFunction:
case RawFunction::kRegularFunction:
case RawFunction::kGetterFunction:
case RawFunction::kSetterFunction: {
ReadUntilFunctionNode(); // read until function node.
- return function.IsImplicitClosureFunction()
- ? BuildGraphOfImplicitClosureFunction(function)
- : BuildGraphOfFunction(is_in_builtin_library_toplevel);
+ if (function.IsImplicitClosureFunction()) {
+ return BuildGraphOfImplicitClosureFunction(function);
+ } else if (function.IsConvertedClosureFunction()) {
+ return BuildGraphOfConvertedClosureFunction(function);
+ }
+ return BuildGraphOfFunction(is_in_builtin_library_toplevel);
}
case RawFunction::kConstructor: {
bool is_factory = function.IsFactory();
@@ -3701,6 +3807,16 @@ Fragment StreamingFlowGraphBuilder::BuildExpression(TokenPosition* position) {
return BuildBoolLiteral(false, position);
case kNullLiteral:
return BuildNullLiteral(position);
+ case kVectorCreation:
+ return BuildVectorCreation(position);
+ case kVectorGet:
+ return BuildVectorGet(position);
+ case kVectorSet:
+ return BuildVectorSet(position);
+ case kVectorCopy:
+ return BuildVectorCopy(position);
+ case kClosureCreation:
+ return BuildClosureCreation(position);
default:
UNREACHABLE();
}
@@ -3856,6 +3972,7 @@ void StreamingFlowGraphBuilder::SkipDartType() {
case kDynamicType:
case kVoidType:
case kBottomType:
+ case kVectorType:
// those contain nothing.
return;
case kInterfaceType:
@@ -3900,8 +4017,8 @@ void StreamingFlowGraphBuilder::SkipInterfaceType(bool simple) {
void StreamingFlowGraphBuilder::SkipFunctionType(bool simple) {
if (!simple) {
SkipTypeParametersList(); // read type_parameters.
- ReadUInt(); // read required parameter count.
- ReadUInt(); // read total parameter count.
+ ReadUInt(); // read required parameter count.
+ ReadUInt(); // read total parameter count.
}
SkipListOfDartTypes(); // read positional_parameters types.
@@ -4050,8 +4167,8 @@ void StreamingFlowGraphBuilder::SkipExpression() {
SkipOptionalDartType(); // read unused static type.
return;
case kStringConcatenation:
- ReadPosition(); // read position.
- SkipListOfExpressions(); // read list of expressions.
+ ReadPosition(); // read position.
+ SkipListOfExpressions(); // read list of expressions.
return;
case kIsExpression:
ReadPosition(); // read position.
@@ -4080,9 +4197,9 @@ void StreamingFlowGraphBuilder::SkipExpression() {
return;
case kListLiteral:
case kConstListLiteral:
- ReadPosition(); // read position.
- SkipDartType(); // read type.
- SkipListOfExpressions(); // read list of expressions.
+ ReadPosition(); // read position.
+ SkipDartType(); // read type.
+ SkipListOfExpressions(); // read list of expressions.
return;
case kMapLiteral:
case kConstMapLiteral: {
@@ -4103,6 +4220,26 @@ void StreamingFlowGraphBuilder::SkipExpression() {
SkipVariableDeclaration(); // read variable declaration.
SkipExpression(); // read expression.
return;
+ case kVectorCreation:
+ ReadUInt(); // read value.
+ return;
+ case kVectorGet:
+ SkipExpression(); // read vector expression.
+ ReadUInt(); // read index.
+ return;
+ case kVectorSet:
+ SkipExpression(); // read vector expression.
+ ReadUInt(); // read index.
+ SkipExpression(); // read value.
+ return;
+ case kVectorCopy:
+ SkipExpression(); // read vector expression.
+ return;
+ case kClosureCreation:
+ SkipCanonicalNameReference(); // read top-level function reference.
+ SkipExpression(); // read context vector.
+ SkipDartType(); // read function type.
+ return;
case kBigIntLiteral:
SkipStringReference(); // read string reference.
return;
@@ -4175,12 +4312,12 @@ void StreamingFlowGraphBuilder::SkipStatement() {
return;
case kForStatement: {
SkipListOfVariableDeclarations(); // read variables.
- Tag tag = ReadTag(); // Read first part of condition.
+ Tag tag = ReadTag(); // Read first part of condition.
if (tag == kSomething) {
SkipExpression(); // read rest of condition.
}
SkipListOfExpressions(); // read updates.
- SkipStatement(); // read body.
+ SkipStatement(); // read body.
return;
}
case kForInStatement:
@@ -4423,6 +4560,10 @@ Value* StreamingFlowGraphBuilder::stack() {
return flow_graph_builder_->stack_;
}
+void StreamingFlowGraphBuilder::Push(Definition* definition) {
+ flow_graph_builder_->Push(definition);
+}
+
Value* StreamingFlowGraphBuilder::Pop() {
return flow_graph_builder_->Pop();
}
@@ -4459,12 +4600,12 @@ intptr_t StreamingFlowGraphBuilder::PeekArgumentsCount() {
intptr_t StreamingFlowGraphBuilder::PeekArgumentsTypeCount() {
AlternativeReadingScope alt(reader_);
- ReadUInt(); // read arguments count.
- return ReadListLength(); // read length of types list.
+ ReadUInt(); // read arguments count.
+ return ReadListLength(); // read length of types list.
}
void StreamingFlowGraphBuilder::SkipArgumentsBeforeActualArguments() {
- ReadUInt(); // read arguments count.
+ ReadUInt(); // read arguments count.
SkipListOfDartTypes(); // read list of types.
}
@@ -4589,6 +4730,20 @@ Fragment StreamingFlowGraphBuilder::AllocateObject(const dart::Class& klass,
return flow_graph_builder_->AllocateObject(klass, argument_count);
}
+Fragment StreamingFlowGraphBuilder::AllocateObject(
+ const dart::Class& klass,
+ const Function& closure_function) {
+ return flow_graph_builder_->AllocateObject(klass, closure_function);
+}
+
+Fragment StreamingFlowGraphBuilder::AllocateContext(int size) {
+ return flow_graph_builder_->AllocateContext(size);
+}
+
+Fragment StreamingFlowGraphBuilder::LoadField(intptr_t offset) {
+ return flow_graph_builder_->LoadField(offset);
+}
+
Fragment StreamingFlowGraphBuilder::InstanceCall(TokenPosition position,
const dart::String& name,
Token::Kind kind,
@@ -4609,6 +4764,11 @@ Fragment StreamingFlowGraphBuilder::StoreStaticField(TokenPosition position,
return flow_graph_builder_->StoreStaticField(position, field);
}
+Fragment StreamingFlowGraphBuilder::StoreInstanceField(TokenPosition position,
+ intptr_t offset) {
+ return flow_graph_builder_->StoreInstanceField(position, offset);
+}
+
Fragment StreamingFlowGraphBuilder::StringInterpolate(TokenPosition position) {
return flow_graph_builder_->StringInterpolate(position);
}
@@ -5519,9 +5679,10 @@ Fragment StreamingFlowGraphBuilder::BuildIsExpression(TokenPosition* p) {
if (dart::FlowGraphBuilder::SimpleInstanceOfType(type)) {
instructions += Constant(type);
instructions += PushArgument(); // Type.
- instructions += InstanceCall(position, dart::Library::PrivateCoreLibName(
- Symbols::_simpleInstanceOf()),
- Token::kIS, 2, 2); // 2 checked arguments.
+ instructions += InstanceCall(
+ position,
+ dart::Library::PrivateCoreLibName(Symbols::_simpleInstanceOf()),
+ Token::kIS, 2, 2); // 2 checked arguments.
return instructions;
}
@@ -5789,8 +5950,8 @@ Fragment StreamingFlowGraphBuilder::BuildFunctionExpression() {
Fragment StreamingFlowGraphBuilder::BuildLet(TokenPosition* position) {
if (position != NULL) *position = TokenPosition::kNoSource;
- Fragment instructions = BuildVariableDeclaration(); // read variable.
- instructions += BuildExpression(); // read body.
+ Fragment instructions = BuildVariableDeclaration(); // read variable.
+ instructions += BuildExpression(); // read body.
return instructions;
}
@@ -5851,6 +6012,93 @@ Fragment StreamingFlowGraphBuilder::BuildNullLiteral(TokenPosition* position) {
return Constant(Instance::ZoneHandle(Z, Instance::null()));
}
+Fragment StreamingFlowGraphBuilder::BuildVectorCreation(
+ TokenPosition* position) {
+ if (position != NULL) *position = TokenPosition::kNoSource;
+
+ intptr_t size = ReadUInt(); // read size.
+ return AllocateContext(size);
+}
+
+Fragment StreamingFlowGraphBuilder::BuildVectorGet(TokenPosition* position) {
+ if (position != NULL) *position = TokenPosition::kNoSource;
+
+ Fragment instructions = BuildExpression(); // read expression.
+ intptr_t index = ReadUInt(); // read index.
+ instructions += LoadField(Context::variable_offset(index));
+ return instructions;
+}
+
+Fragment StreamingFlowGraphBuilder::BuildVectorSet(TokenPosition* position) {
+ if (position != NULL) *position = TokenPosition::kNoSource;
+
+ Fragment instructions = BuildExpression(); // read vector expression.
+ intptr_t index = ReadUInt(); // read index.
+ instructions += BuildExpression(); // read value expression.
+
+ // The assigned value is the result of the expression.
+ LocalVariable* result = MakeTemporary();
kustermann 2017/06/20 12:13:14 I suspect this is unsafe: The MakeTemporary() mak
Dmitry Stefantsov 2017/06/22 14:12:52 Thank you so much for pointing me to that! Indeed
+
+ Value* value = Pop();
+ StoreInstanceFieldInstr* store = new (Z)
+ StoreInstanceFieldInstr(Context::variable_offset(index), Pop(), value,
+ kNoStoreBarrier, TokenPosition::kNoSource);
+ instructions <<= store;
+
+ // Load the result that is stored in the temporary variable.
+ instructions += LoadLocal(result);
+
+ return instructions;
+}
+
+Fragment StreamingFlowGraphBuilder::BuildVectorCopy(TokenPosition* position) {
+ if (position != NULL) *position = TokenPosition::kNoSource;
+
+ Fragment instructions = BuildExpression(); // read vector expression.
+ CloneContextInstr* clone_instruction = new (Z) CloneContextInstr(
+ TokenPosition::kNoSource, Pop(), Thread::Current()->GetNextDeoptId());
kustermann 2017/06/20 12:13:13 Maybe introduce local variables, so one knows what
Dmitry Stefantsov 2017/06/22 14:12:52 Thanks! That would read better indeed. Done.
+ instructions <<= clone_instruction;
+ Push(clone_instruction);
+
+ return instructions;
+}
+
+Fragment StreamingFlowGraphBuilder::BuildClosureCreation(
+ TokenPosition* position) {
+ if (position != NULL) *position = TokenPosition::kNoSource;
+
+ NameIndex function_reference =
+ ReadCanonicalNameReference(); // read function reference.
+ Function& function = Function::ZoneHandle(
+ Z, H.LookupStaticMethodByKernelProcedure(function_reference));
+ function = function.ConvertedClosureFunction();
+ ASSERT(!function.IsNull());
+
+ const dart::Class& closure_class =
+ dart::Class::ZoneHandle(Z, I->object_store()->closure_class());
+ Fragment instructions = AllocateObject(closure_class, function);
+ LocalVariable* closure = MakeTemporary();
kustermann 2017/06/20 12:13:14 Here for example (as opposed to above), the [closu
Dmitry Stefantsov 2017/06/22 14:12:52 Yep. Now I see it. Thanks!
+
+ instructions += BuildExpression(); // read context vector.
+ LocalVariable* context = MakeTemporary();
+
+ instructions += LoadLocal(closure);
+ instructions += Constant(function);
+ instructions +=
+ StoreInstanceField(TokenPosition::kNoSource, Closure::function_offset());
+
+ instructions += LoadLocal(closure);
+ instructions += LoadLocal(context);
+ instructions +=
+ StoreInstanceField(TokenPosition::kNoSource, Closure::context_offset());
+
+ instructions += Drop();
+
+ SkipDartType(); // skip function type of the closure.
+
+ return instructions;
+}
+
Fragment StreamingFlowGraphBuilder::BuildInvalidStatement() {
H.ReportError("Invalid statements not implemented yet!");
return Fragment();

Powered by Google App Engine
This is Rietveld 408576698