From e70dec4e82efdf843c76cedbcbaaa29be3777b7d Mon Sep 17 00:00:00 2001 From: Alexander Markov Date: Mon, 24 Oct 2022 23:00:47 +0000 Subject: [PATCH] [vm] Allocation sinking of records This change adds all necessary support for allocation sinking and materialization of record instances. TEST=vm/cc/AllocationSinking_Records Issue: https://github.com/dart-lang/sdk/issues/49719 Change-Id: I040ce8b1ed3220f87a767b590050de3e50573170 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/265380 Reviewed-by: Ryan Macnak Commit-Queue: Alexander Markov Reviewed-by: Slava Egorov --- pkg/test_runner/lib/src/test_suite.dart | 6 +- runtime/vm/compiler/backend/il.cc | 5 +- runtime/vm/compiler/backend/il.h | 27 +++-- .../backend/redundancy_elimination.cc | 4 + .../backend/redundancy_elimination_test.cc | 102 ++++++++++++++++++ .../frontend/base_flow_graph_builder.cc | 4 +- .../frontend/base_flow_graph_builder.h | 4 +- .../frontend/kernel_binary_flowgraph.cc | 3 +- runtime/vm/deferred_objects.cc | 43 +++++++- runtime/vm/object.h | 8 ++ 10 files changed, 183 insertions(+), 23 deletions(-) diff --git a/pkg/test_runner/lib/src/test_suite.dart b/pkg/test_runner/lib/src/test_suite.dart index 517fb036db5..1880a6d93b6 100644 --- a/pkg/test_runner/lib/src/test_suite.dart +++ b/pkg/test_runner/lib/src/test_suite.dart @@ -332,13 +332,15 @@ class VMTestSuite extends TestSuite { ? '$buildDir/gen/kernel-service.dart.snapshot' : '$buildDir/gen/kernel_service.dill'; var dfePath = Path(filename).absolute.toNativePath(); + // Enable 'records' experiment as it is used by certain vm/cc unit tests. + final experiments = [...configuration.experiments, 'records']; var args = [ ...initialTargetArguments, // '--dfe' must be the first VM argument for run_vm_test to pick it up. '--dfe=$dfePath', if (expectations.contains(Expectation.crash)) '--suppress-core-dump', - if (configuration.experiments.isNotEmpty) - '--enable-experiment=${configuration.experiments.join(",")}', + if (experiments.isNotEmpty) + '--enable-experiment=${experiments.join(",")}', if (configuration.nnbdMode == NnbdMode.strong) '--sound-null-safety', ...configuration.standardOptions, ...configuration.vmOptions, diff --git a/runtime/vm/compiler/backend/il.cc b/runtime/vm/compiler/backend/il.cc index e0fbcc4b547..950b06c3be0 100644 --- a/runtime/vm/compiler/backend/il.cc +++ b/runtime/vm/compiler/backend/il.cc @@ -7660,10 +7660,12 @@ void SuspendInstr::EmitNativeCode(FlowGraphCompiler* compiler) { LocationSummary* AllocateRecordInstr::MakeLocationSummary(Zone* zone, bool opt) const { - const intptr_t kNumInputs = 0; + const intptr_t kNumInputs = 1; const intptr_t kNumTemps = 0; LocationSummary* locs = new (zone) LocationSummary(zone, kNumInputs, kNumTemps, LocationSummary::kCall); + locs->set_in(0, + Location::RegisterLocation(AllocateRecordABI::kFieldNamesReg)); locs->set_out(0, Location::RegisterLocation(AllocateRecordABI::kResultReg)); return locs; } @@ -7673,7 +7675,6 @@ void AllocateRecordInstr::EmitNativeCode(FlowGraphCompiler* compiler) { compiler->zone(), compiler->isolate_group()->object_store()->allocate_record_stub()); __ LoadImmediate(AllocateRecordABI::kNumFieldsReg, num_fields()); - __ LoadObject(AllocateRecordABI::kFieldNamesReg, field_names()); compiler->GenerateStubCall(source(), stub, UntaggedPcDescriptors::kOther, locs(), deopt_id(), env()); } diff --git a/runtime/vm/compiler/backend/il.h b/runtime/vm/compiler/backend/il.h index 8ddc06d392d..ee98b8e85d5 100644 --- a/runtime/vm/compiler/backend/il.h +++ b/runtime/vm/compiler/backend/il.h @@ -6982,24 +6982,31 @@ class AllocateUninitializedContextInstr : public TemplateAllocation<0> { }; // Allocates and null initializes a record object. -class AllocateRecordInstr : public TemplateAllocation<0> { +class AllocateRecordInstr : public TemplateAllocation<1> { public: + enum { kFieldNamesPos = 0 }; AllocateRecordInstr(const InstructionSource& source, intptr_t num_fields, - const Array& field_names, + Value* field_names, intptr_t deopt_id) - : TemplateAllocation(source, deopt_id), - num_fields_(num_fields), - field_names_(field_names) { - ASSERT(field_names.IsNotTemporaryScopedHandle()); - ASSERT(field_names.IsCanonical()); + : TemplateAllocation(source, deopt_id), num_fields_(num_fields) { + SetInputAt(kFieldNamesPos, field_names); } DECLARE_INSTRUCTION(AllocateRecord) virtual CompileType ComputeType() const; intptr_t num_fields() const { return num_fields_; } - const Array& field_names() const { return field_names_; } + Value* field_names() const { return InputAt(kFieldNamesPos); } + + virtual const Slot* SlotForInput(intptr_t pos) { + switch (pos) { + case kFieldNamesPos: + return &Slot::Record_field_names(); + default: + return TemplateAllocation::SlotForInput(pos); + } + } virtual bool HasUnknownSideEffects() const { return false; } @@ -7008,9 +7015,7 @@ class AllocateRecordInstr : public TemplateAllocation<0> { compiler::target::Record::InstanceSize(num_fields_)); } -#define FIELD_LIST(F) \ - F(const intptr_t, num_fields_) \ - F(const Array&, field_names_) +#define FIELD_LIST(F) F(const intptr_t, num_fields_) DECLARE_INSTRUCTION_SERIALIZABLE_FIELDS(AllocateRecordInstr, TemplateAllocation, diff --git a/runtime/vm/compiler/backend/redundancy_elimination.cc b/runtime/vm/compiler/backend/redundancy_elimination.cc index 42edaafed7b..66c8ccbc97c 100644 --- a/runtime/vm/compiler/backend/redundancy_elimination.cc +++ b/runtime/vm/compiler/backend/redundancy_elimination.cc @@ -3855,6 +3855,10 @@ void AllocationSinking::CreateMaterializationAt( cls = &Class::ZoneHandle( flow_graph_->isolate_group()->class_table()->At(instr->class_id())); num_elements = instr->GetConstantNumElements(); + } else if (auto instr = alloc->AsAllocateRecord()) { + cls = &Class::ZoneHandle( + flow_graph_->isolate_group()->class_table()->At(kRecordCid)); + num_elements = instr->num_fields(); } else { UNREACHABLE(); } diff --git a/runtime/vm/compiler/backend/redundancy_elimination_test.cc b/runtime/vm/compiler/backend/redundancy_elimination_test.cc index 45181307af3..c3f57b50628 100644 --- a/runtime/vm/compiler/backend/redundancy_elimination_test.cc +++ b/runtime/vm/compiler/backend/redundancy_elimination_test.cc @@ -1366,6 +1366,108 @@ main() { EXPECT(string_interpolate->ArgumentAt(0) == create_array); } +ISOLATE_UNIT_TEST_CASE(AllocationSinking_Records) { + const char* kScript = R"( + +@pragma('vm:prefer-inline') +({int field1, String field2}) getRecord(int x, String y) => + (field1: x, field2: y); + +@pragma('vm:never-inline') +String foo(int x, String y) { + // All allocations in this function are eliminated by the compiler, + // except array allocation for string interpolation at the end. + (int, bool) r1 = (x, true); + final r2 = getRecord(x, y); + int sum = r1.$0 + r2.field1; + return "r1: (${r1.$0}, ${r1.$1}), " + "r2: (field1: ${r2.field1}, field2: ${r2.field2}), sum: $sum"; +} + +int count = 0; +main() { + // Deoptimize on the 2nd run. + return foo(count++ == 0 ? 42 : 9223372036854775807, 'hey'); +} + )"; + + const auto& root_library = Library::Handle(LoadTestScript(kScript)); + const auto& result1 = Object::Handle(Invoke(root_library, "main")); + EXPECT(result1.IsString()); + EXPECT_STREQ(result1.ToCString(), + "r1: (42, true), r2: (field1: 42, field2: hey), sum: 84"); + const auto& function = Function::Handle(GetFunction(root_library, "foo")); + TestPipeline pipeline(function, CompilerPass::kJIT); + FlowGraph* flow_graph = pipeline.RunPasses({}); + ASSERT(flow_graph != nullptr); + + auto entry = flow_graph->graph_entry()->normal_entry(); + EXPECT(entry != nullptr); + + /* Flow graph to match: + + 2: B1[function entry]:2 { + v2 <- Parameter(0) [-9223372036854775808, 9223372036854775807] T{int} + v3 <- Parameter(1) T{String} +} + 4: CheckStackOverflow:8(stack=0, loop=0) + 5: ParallelMove rax <- S+3 + 6: CheckSmi:16(v2) + 8: ParallelMove rcx <- rax + 8: v9 <- BinarySmiOp:16(+, v2 T{_Smi}, v2 T{_Smi}) [-4611686018427387904, 4611686018427387903] T{_Smi} + 9: ParallelMove rbx <- C, r10 <- C, S-3 <- rcx + 10: v11 <- CreateArray:18(v0, v10) T{_List} + 11: ParallelMove rax <- rax + 12: StoreIndexed(v11, v12, v13, NoStoreBarrier) + 13: ParallelMove rcx <- S+3 + 14: StoreIndexed(v11, v14, v2 T{_Smi}, NoStoreBarrier) + 16: StoreIndexed(v11, v16, v17, NoStoreBarrier) + 18: StoreIndexed(v11, v18, v5, NoStoreBarrier) + 20: StoreIndexed(v11, v20, v21, NoStoreBarrier) + 22: StoreIndexed(v11, v22, v2 T{_Smi}, NoStoreBarrier) + 24: StoreIndexed(v11, v24, v25, NoStoreBarrier) + 25: ParallelMove rcx <- S+2 + 26: StoreIndexed(v11, v26, v3, NoStoreBarrier) + 28: StoreIndexed(v11, v28, v29, NoStoreBarrier) + 29: ParallelMove rcx <- S-3 + 30: StoreIndexed(v11, v30, v9, NoStoreBarrier) + 32: PushArgument(v11) + 34: v31 <- StaticCall:20( _interpolate@0150898<0> v11, recognized_kind = StringBaseInterpolate) T{String} + 35: ParallelMove rax <- rax + 36: Return:24(v31) +*/ + + ILMatcher cursor(flow_graph, entry, /*trace=*/true, + ParallelMovesHandling::kSkip); + RELEASE_ASSERT(cursor.TryMatch({ + kMatchAndMoveFunctionEntry, + kMatchAndMoveCheckStackOverflow, + kMatchAndMoveCheckSmi, + kMatchAndMoveBinarySmiOp, + kMatchAndMoveCreateArray, + kMatchAndMoveStoreIndexed, + kMatchAndMoveStoreIndexed, + kMatchAndMoveStoreIndexed, + kMatchAndMoveStoreIndexed, + kMatchAndMoveStoreIndexed, + kMatchAndMoveStoreIndexed, + kMatchAndMoveStoreIndexed, + kMatchAndMoveStoreIndexed, + kMatchAndMoveStoreIndexed, + kMatchAndMoveStoreIndexed, + kMatchAndMovePushArgument, + kMatchAndMoveStaticCall, + kMatchReturn, + })); + + Compiler::CompileOptimizedFunction(thread, function); + const auto& result2 = Object::Handle(Invoke(root_library, "main")); + EXPECT(result2.IsString()); + EXPECT_STREQ(result2.ToCString(), + "r1: (9223372036854775807, true), r2: (field1: " + "9223372036854775807, field2: hey), sum: -2"); +} + #if !defined(TARGET_ARCH_IA32) ISOLATE_UNIT_TEST_CASE(DelayAllocations_DelayAcrossCalls) { diff --git a/runtime/vm/compiler/frontend/base_flow_graph_builder.cc b/runtime/vm/compiler/frontend/base_flow_graph_builder.cc index 4e7ec53b7b0..37849625e0a 100644 --- a/runtime/vm/compiler/frontend/base_flow_graph_builder.cc +++ b/runtime/vm/compiler/frontend/base_flow_graph_builder.cc @@ -940,8 +940,8 @@ Fragment BaseFlowGraphBuilder::CreateArray() { } Fragment BaseFlowGraphBuilder::AllocateRecord(TokenPosition position, - intptr_t num_fields, - const Array& field_names) { + intptr_t num_fields) { + Value* field_names = Pop(); AllocateRecordInstr* allocate = new (Z) AllocateRecordInstr( InstructionSource(position), num_fields, field_names, GetNextDeoptId()); Push(allocate); diff --git a/runtime/vm/compiler/frontend/base_flow_graph_builder.h b/runtime/vm/compiler/frontend/base_flow_graph_builder.h index 95d297c1986..9282ed2b1e4 100644 --- a/runtime/vm/compiler/frontend/base_flow_graph_builder.h +++ b/runtime/vm/compiler/frontend/base_flow_graph_builder.h @@ -353,9 +353,7 @@ class BaseFlowGraphBuilder { // Top of the stack should be the closure function. Fragment AllocateClosure(TokenPosition position = TokenPosition::kNoSource); Fragment CreateArray(); - Fragment AllocateRecord(TokenPosition position, - intptr_t num_fields, - const Array& field_names); + Fragment AllocateRecord(TokenPosition position, intptr_t num_fields); Fragment AllocateTypedData(TokenPosition position, classid_t class_id); Fragment InstantiateType(const AbstractType& type); Fragment InstantiateTypeArguments(const TypeArguments& type_arguments); diff --git a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc index dd866730560..622b18babe3 100644 --- a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc +++ b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc @@ -4066,7 +4066,8 @@ Fragment StreamingFlowGraphBuilder::BuildRecordLiteral(TokenPosition* p) { // records. Fragment instructions; - instructions += B->AllocateRecord(position, num_fields, *field_names); + instructions += Constant(*field_names); + instructions += B->AllocateRecord(position, num_fields); LocalVariable* record = MakeTemporary(); // List of positional. diff --git a/runtime/vm/deferred_objects.cc b/runtime/vm/deferred_objects.cc index b64d7dd2a48..77108660d07 100644 --- a/runtime/vm/deferred_objects.cc +++ b/runtime/vm/deferred_objects.cc @@ -247,6 +247,17 @@ void DeferredObject::Create() { } object_ = &Array::ZoneHandle(Array::New(num_elements)); } break; + case kRecordCid: { + const intptr_t num_fields = + Smi::Cast(Object::Handle(GetLength())).Value(); + if (FLAG_trace_deoptimization_verbose) { + OS::PrintErr("materializing record of length %" Pd " (%" Px ", %" Pd + " fields)\n", + num_fields, reinterpret_cast(args_), field_count_); + } + object_ = + &Record::ZoneHandle(Record::New(num_fields, Object::empty_array())); + } break; default: if (IsTypedDataClassId(cls.id())) { const intptr_t num_elements = @@ -301,7 +312,7 @@ void DeferredObject::Fill() { context.set_parent(parent); if (FLAG_trace_deoptimization_verbose) { OS::PrintErr(" ctx@parent (offset %" Pd ") <- %s\n", - offset.Value(), value.ToCString()); + offset.Value(), parent.ToCString()); } } else { intptr_t context_index = ToContextIndex(offset.Value()); @@ -328,7 +339,7 @@ void DeferredObject::Fill() { array.SetTypeArguments(type_args); if (FLAG_trace_deoptimization_verbose) { OS::PrintErr(" array@type_args (offset %" Pd ") <- %s\n", - offset.Value(), value.ToCString()); + offset.Value(), type_args.ToCString()); } } else { const intptr_t index = Array::index_at_offset(offset.Value()); @@ -341,6 +352,34 @@ void DeferredObject::Fill() { } } } break; + case kRecordCid: { + const Record& record = Record::Cast(*object_); + + Smi& offset = Smi::Handle(); + Object& value = Object::Handle(); + + for (intptr_t i = 0; i < field_count_; i++) { + offset ^= GetFieldOffset(i); + if (offset.Value() == Record::field_names_offset()) { + // Copy field_names. + Array& field_names = Array::Handle(); + field_names ^= GetValue(i); + record.set_field_names(field_names); + if (FLAG_trace_deoptimization_verbose) { + OS::PrintErr(" record@field_names (offset %" Pd ") <- %s\n", + offset.Value(), field_names.ToCString()); + } + } else { + const intptr_t index = Record::field_index_at_offset(offset.Value()); + value = GetValue(i); + record.SetFieldAt(index, value); + if (FLAG_trace_deoptimization_verbose) { + OS::PrintErr(" record@%" Pd " (offset %" Pd ") <- %s\n", index, + offset.Value(), value.ToCString()); + } + } + } + } break; default: if (IsTypedDataClassId(cls.id())) { const TypedData& typed_data = TypedData::Cast(*object_); diff --git a/runtime/vm/object.h b/runtime/vm/object.h index c6ad5cd11aa..d2cc89f4060 100644 --- a/runtime/vm/object.h +++ b/runtime/vm/object.h @@ -10784,6 +10784,13 @@ class Record : public Instance { return OFFSET_OF_RETURNED_VALUE(UntaggedRecord, data) + kBytesPerElement * index; } + static intptr_t field_index_at_offset(intptr_t offset_in_bytes) { + const intptr_t index = + (offset_in_bytes - OFFSET_OF_RETURNED_VALUE(UntaggedRecord, data)) / + kBytesPerElement; + ASSERT(index >= 0); + return index; + } static intptr_t InstanceSize() { ASSERT(sizeof(UntaggedRecord) == @@ -10826,6 +10833,7 @@ class Record : public Instance { FINAL_HEAP_OBJECT_IMPLEMENTATION(Record, Instance); friend class Class; + friend class DeferredObject; // For set_field_names. friend class Object; };