Skip to content

Commit e70dec4

Browse files
alexmarkovCommit Queue
authored and
Commit Queue
committed
[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: #49719 Change-Id: I040ce8b1ed3220f87a767b590050de3e50573170 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/265380 Reviewed-by: Ryan Macnak <[email protected]> Commit-Queue: Alexander Markov <[email protected]> Reviewed-by: Slava Egorov <[email protected]>
1 parent 0ad53cb commit e70dec4

File tree

10 files changed

+183
-23
lines changed

10 files changed

+183
-23
lines changed

pkg/test_runner/lib/src/test_suite.dart

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -332,13 +332,15 @@ class VMTestSuite extends TestSuite {
332332
? '$buildDir/gen/kernel-service.dart.snapshot'
333333
: '$buildDir/gen/kernel_service.dill';
334334
var dfePath = Path(filename).absolute.toNativePath();
335+
// Enable 'records' experiment as it is used by certain vm/cc unit tests.
336+
final experiments = [...configuration.experiments, 'records'];
335337
var args = [
336338
...initialTargetArguments,
337339
// '--dfe' must be the first VM argument for run_vm_test to pick it up.
338340
'--dfe=$dfePath',
339341
if (expectations.contains(Expectation.crash)) '--suppress-core-dump',
340-
if (configuration.experiments.isNotEmpty)
341-
'--enable-experiment=${configuration.experiments.join(",")}',
342+
if (experiments.isNotEmpty)
343+
'--enable-experiment=${experiments.join(",")}',
342344
if (configuration.nnbdMode == NnbdMode.strong) '--sound-null-safety',
343345
...configuration.standardOptions,
344346
...configuration.vmOptions,

runtime/vm/compiler/backend/il.cc

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7660,10 +7660,12 @@ void SuspendInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
76607660

76617661
LocationSummary* AllocateRecordInstr::MakeLocationSummary(Zone* zone,
76627662
bool opt) const {
7663-
const intptr_t kNumInputs = 0;
7663+
const intptr_t kNumInputs = 1;
76647664
const intptr_t kNumTemps = 0;
76657665
LocationSummary* locs = new (zone)
76667666
LocationSummary(zone, kNumInputs, kNumTemps, LocationSummary::kCall);
7667+
locs->set_in(0,
7668+
Location::RegisterLocation(AllocateRecordABI::kFieldNamesReg));
76677669
locs->set_out(0, Location::RegisterLocation(AllocateRecordABI::kResultReg));
76687670
return locs;
76697671
}
@@ -7673,7 +7675,6 @@ void AllocateRecordInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
76737675
compiler->zone(),
76747676
compiler->isolate_group()->object_store()->allocate_record_stub());
76757677
__ LoadImmediate(AllocateRecordABI::kNumFieldsReg, num_fields());
7676-
__ LoadObject(AllocateRecordABI::kFieldNamesReg, field_names());
76777678
compiler->GenerateStubCall(source(), stub, UntaggedPcDescriptors::kOther,
76787679
locs(), deopt_id(), env());
76797680
}

runtime/vm/compiler/backend/il.h

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6982,24 +6982,31 @@ class AllocateUninitializedContextInstr : public TemplateAllocation<0> {
69826982
};
69836983

69846984
// Allocates and null initializes a record object.
6985-
class AllocateRecordInstr : public TemplateAllocation<0> {
6985+
class AllocateRecordInstr : public TemplateAllocation<1> {
69866986
public:
6987+
enum { kFieldNamesPos = 0 };
69876988
AllocateRecordInstr(const InstructionSource& source,
69886989
intptr_t num_fields,
6989-
const Array& field_names,
6990+
Value* field_names,
69906991
intptr_t deopt_id)
6991-
: TemplateAllocation(source, deopt_id),
6992-
num_fields_(num_fields),
6993-
field_names_(field_names) {
6994-
ASSERT(field_names.IsNotTemporaryScopedHandle());
6995-
ASSERT(field_names.IsCanonical());
6992+
: TemplateAllocation(source, deopt_id), num_fields_(num_fields) {
6993+
SetInputAt(kFieldNamesPos, field_names);
69966994
}
69976995

69986996
DECLARE_INSTRUCTION(AllocateRecord)
69996997
virtual CompileType ComputeType() const;
70006998

70016999
intptr_t num_fields() const { return num_fields_; }
7002-
const Array& field_names() const { return field_names_; }
7000+
Value* field_names() const { return InputAt(kFieldNamesPos); }
7001+
7002+
virtual const Slot* SlotForInput(intptr_t pos) {
7003+
switch (pos) {
7004+
case kFieldNamesPos:
7005+
return &Slot::Record_field_names();
7006+
default:
7007+
return TemplateAllocation::SlotForInput(pos);
7008+
}
7009+
}
70037010

70047011
virtual bool HasUnknownSideEffects() const { return false; }
70057012

@@ -7008,9 +7015,7 @@ class AllocateRecordInstr : public TemplateAllocation<0> {
70087015
compiler::target::Record::InstanceSize(num_fields_));
70097016
}
70107017

7011-
#define FIELD_LIST(F) \
7012-
F(const intptr_t, num_fields_) \
7013-
F(const Array&, field_names_)
7018+
#define FIELD_LIST(F) F(const intptr_t, num_fields_)
70147019

70157020
DECLARE_INSTRUCTION_SERIALIZABLE_FIELDS(AllocateRecordInstr,
70167021
TemplateAllocation,

runtime/vm/compiler/backend/redundancy_elimination.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3855,6 +3855,10 @@ void AllocationSinking::CreateMaterializationAt(
38553855
cls = &Class::ZoneHandle(
38563856
flow_graph_->isolate_group()->class_table()->At(instr->class_id()));
38573857
num_elements = instr->GetConstantNumElements();
3858+
} else if (auto instr = alloc->AsAllocateRecord()) {
3859+
cls = &Class::ZoneHandle(
3860+
flow_graph_->isolate_group()->class_table()->At(kRecordCid));
3861+
num_elements = instr->num_fields();
38583862
} else {
38593863
UNREACHABLE();
38603864
}

runtime/vm/compiler/backend/redundancy_elimination_test.cc

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1366,6 +1366,108 @@ main() {
13661366
EXPECT(string_interpolate->ArgumentAt(0) == create_array);
13671367
}
13681368

1369+
ISOLATE_UNIT_TEST_CASE(AllocationSinking_Records) {
1370+
const char* kScript = R"(
1371+
1372+
@pragma('vm:prefer-inline')
1373+
({int field1, String field2}) getRecord(int x, String y) =>
1374+
(field1: x, field2: y);
1375+
1376+
@pragma('vm:never-inline')
1377+
String foo(int x, String y) {
1378+
// All allocations in this function are eliminated by the compiler,
1379+
// except array allocation for string interpolation at the end.
1380+
(int, bool) r1 = (x, true);
1381+
final r2 = getRecord(x, y);
1382+
int sum = r1.$0 + r2.field1;
1383+
return "r1: (${r1.$0}, ${r1.$1}), "
1384+
"r2: (field1: ${r2.field1}, field2: ${r2.field2}), sum: $sum";
1385+
}
1386+
1387+
int count = 0;
1388+
main() {
1389+
// Deoptimize on the 2nd run.
1390+
return foo(count++ == 0 ? 42 : 9223372036854775807, 'hey');
1391+
}
1392+
)";
1393+
1394+
const auto& root_library = Library::Handle(LoadTestScript(kScript));
1395+
const auto& result1 = Object::Handle(Invoke(root_library, "main"));
1396+
EXPECT(result1.IsString());
1397+
EXPECT_STREQ(result1.ToCString(),
1398+
"r1: (42, true), r2: (field1: 42, field2: hey), sum: 84");
1399+
const auto& function = Function::Handle(GetFunction(root_library, "foo"));
1400+
TestPipeline pipeline(function, CompilerPass::kJIT);
1401+
FlowGraph* flow_graph = pipeline.RunPasses({});
1402+
ASSERT(flow_graph != nullptr);
1403+
1404+
auto entry = flow_graph->graph_entry()->normal_entry();
1405+
EXPECT(entry != nullptr);
1406+
1407+
/* Flow graph to match:
1408+
1409+
2: B1[function entry]:2 {
1410+
v2 <- Parameter(0) [-9223372036854775808, 9223372036854775807] T{int}
1411+
v3 <- Parameter(1) T{String}
1412+
}
1413+
4: CheckStackOverflow:8(stack=0, loop=0)
1414+
5: ParallelMove rax <- S+3
1415+
6: CheckSmi:16(v2)
1416+
8: ParallelMove rcx <- rax
1417+
8: v9 <- BinarySmiOp:16(+, v2 T{_Smi}, v2 T{_Smi}) [-4611686018427387904, 4611686018427387903] T{_Smi}
1418+
9: ParallelMove rbx <- C, r10 <- C, S-3 <- rcx
1419+
10: v11 <- CreateArray:18(v0, v10) T{_List}
1420+
11: ParallelMove rax <- rax
1421+
12: StoreIndexed(v11, v12, v13, NoStoreBarrier)
1422+
13: ParallelMove rcx <- S+3
1423+
14: StoreIndexed(v11, v14, v2 T{_Smi}, NoStoreBarrier)
1424+
16: StoreIndexed(v11, v16, v17, NoStoreBarrier)
1425+
18: StoreIndexed(v11, v18, v5, NoStoreBarrier)
1426+
20: StoreIndexed(v11, v20, v21, NoStoreBarrier)
1427+
22: StoreIndexed(v11, v22, v2 T{_Smi}, NoStoreBarrier)
1428+
24: StoreIndexed(v11, v24, v25, NoStoreBarrier)
1429+
25: ParallelMove rcx <- S+2
1430+
26: StoreIndexed(v11, v26, v3, NoStoreBarrier)
1431+
28: StoreIndexed(v11, v28, v29, NoStoreBarrier)
1432+
29: ParallelMove rcx <- S-3
1433+
30: StoreIndexed(v11, v30, v9, NoStoreBarrier)
1434+
32: PushArgument(v11)
1435+
34: v31 <- StaticCall:20( _interpolate@0150898<0> v11, recognized_kind = StringBaseInterpolate) T{String}
1436+
35: ParallelMove rax <- rax
1437+
36: Return:24(v31)
1438+
*/
1439+
1440+
ILMatcher cursor(flow_graph, entry, /*trace=*/true,
1441+
ParallelMovesHandling::kSkip);
1442+
RELEASE_ASSERT(cursor.TryMatch({
1443+
kMatchAndMoveFunctionEntry,
1444+
kMatchAndMoveCheckStackOverflow,
1445+
kMatchAndMoveCheckSmi,
1446+
kMatchAndMoveBinarySmiOp,
1447+
kMatchAndMoveCreateArray,
1448+
kMatchAndMoveStoreIndexed,
1449+
kMatchAndMoveStoreIndexed,
1450+
kMatchAndMoveStoreIndexed,
1451+
kMatchAndMoveStoreIndexed,
1452+
kMatchAndMoveStoreIndexed,
1453+
kMatchAndMoveStoreIndexed,
1454+
kMatchAndMoveStoreIndexed,
1455+
kMatchAndMoveStoreIndexed,
1456+
kMatchAndMoveStoreIndexed,
1457+
kMatchAndMoveStoreIndexed,
1458+
kMatchAndMovePushArgument,
1459+
kMatchAndMoveStaticCall,
1460+
kMatchReturn,
1461+
}));
1462+
1463+
Compiler::CompileOptimizedFunction(thread, function);
1464+
const auto& result2 = Object::Handle(Invoke(root_library, "main"));
1465+
EXPECT(result2.IsString());
1466+
EXPECT_STREQ(result2.ToCString(),
1467+
"r1: (9223372036854775807, true), r2: (field1: "
1468+
"9223372036854775807, field2: hey), sum: -2");
1469+
}
1470+
13691471
#if !defined(TARGET_ARCH_IA32)
13701472

13711473
ISOLATE_UNIT_TEST_CASE(DelayAllocations_DelayAcrossCalls) {

runtime/vm/compiler/frontend/base_flow_graph_builder.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -940,8 +940,8 @@ Fragment BaseFlowGraphBuilder::CreateArray() {
940940
}
941941

942942
Fragment BaseFlowGraphBuilder::AllocateRecord(TokenPosition position,
943-
intptr_t num_fields,
944-
const Array& field_names) {
943+
intptr_t num_fields) {
944+
Value* field_names = Pop();
945945
AllocateRecordInstr* allocate = new (Z) AllocateRecordInstr(
946946
InstructionSource(position), num_fields, field_names, GetNextDeoptId());
947947
Push(allocate);

runtime/vm/compiler/frontend/base_flow_graph_builder.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -353,9 +353,7 @@ class BaseFlowGraphBuilder {
353353
// Top of the stack should be the closure function.
354354
Fragment AllocateClosure(TokenPosition position = TokenPosition::kNoSource);
355355
Fragment CreateArray();
356-
Fragment AllocateRecord(TokenPosition position,
357-
intptr_t num_fields,
358-
const Array& field_names);
356+
Fragment AllocateRecord(TokenPosition position, intptr_t num_fields);
359357
Fragment AllocateTypedData(TokenPosition position, classid_t class_id);
360358
Fragment InstantiateType(const AbstractType& type);
361359
Fragment InstantiateTypeArguments(const TypeArguments& type_arguments);

runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4066,7 +4066,8 @@ Fragment StreamingFlowGraphBuilder::BuildRecordLiteral(TokenPosition* p) {
40664066
// records.
40674067

40684068
Fragment instructions;
4069-
instructions += B->AllocateRecord(position, num_fields, *field_names);
4069+
instructions += Constant(*field_names);
4070+
instructions += B->AllocateRecord(position, num_fields);
40704071
LocalVariable* record = MakeTemporary();
40714072

40724073
// List of positional.

runtime/vm/deferred_objects.cc

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,17 @@ void DeferredObject::Create() {
247247
}
248248
object_ = &Array::ZoneHandle(Array::New(num_elements));
249249
} break;
250+
case kRecordCid: {
251+
const intptr_t num_fields =
252+
Smi::Cast(Object::Handle(GetLength())).Value();
253+
if (FLAG_trace_deoptimization_verbose) {
254+
OS::PrintErr("materializing record of length %" Pd " (%" Px ", %" Pd
255+
" fields)\n",
256+
num_fields, reinterpret_cast<uword>(args_), field_count_);
257+
}
258+
object_ =
259+
&Record::ZoneHandle(Record::New(num_fields, Object::empty_array()));
260+
} break;
250261
default:
251262
if (IsTypedDataClassId(cls.id())) {
252263
const intptr_t num_elements =
@@ -301,7 +312,7 @@ void DeferredObject::Fill() {
301312
context.set_parent(parent);
302313
if (FLAG_trace_deoptimization_verbose) {
303314
OS::PrintErr(" ctx@parent (offset %" Pd ") <- %s\n",
304-
offset.Value(), value.ToCString());
315+
offset.Value(), parent.ToCString());
305316
}
306317
} else {
307318
intptr_t context_index = ToContextIndex(offset.Value());
@@ -328,7 +339,7 @@ void DeferredObject::Fill() {
328339
array.SetTypeArguments(type_args);
329340
if (FLAG_trace_deoptimization_verbose) {
330341
OS::PrintErr(" array@type_args (offset %" Pd ") <- %s\n",
331-
offset.Value(), value.ToCString());
342+
offset.Value(), type_args.ToCString());
332343
}
333344
} else {
334345
const intptr_t index = Array::index_at_offset(offset.Value());
@@ -341,6 +352,34 @@ void DeferredObject::Fill() {
341352
}
342353
}
343354
} break;
355+
case kRecordCid: {
356+
const Record& record = Record::Cast(*object_);
357+
358+
Smi& offset = Smi::Handle();
359+
Object& value = Object::Handle();
360+
361+
for (intptr_t i = 0; i < field_count_; i++) {
362+
offset ^= GetFieldOffset(i);
363+
if (offset.Value() == Record::field_names_offset()) {
364+
// Copy field_names.
365+
Array& field_names = Array::Handle();
366+
field_names ^= GetValue(i);
367+
record.set_field_names(field_names);
368+
if (FLAG_trace_deoptimization_verbose) {
369+
OS::PrintErr(" record@field_names (offset %" Pd ") <- %s\n",
370+
offset.Value(), field_names.ToCString());
371+
}
372+
} else {
373+
const intptr_t index = Record::field_index_at_offset(offset.Value());
374+
value = GetValue(i);
375+
record.SetFieldAt(index, value);
376+
if (FLAG_trace_deoptimization_verbose) {
377+
OS::PrintErr(" record@%" Pd " (offset %" Pd ") <- %s\n", index,
378+
offset.Value(), value.ToCString());
379+
}
380+
}
381+
}
382+
} break;
344383
default:
345384
if (IsTypedDataClassId(cls.id())) {
346385
const TypedData& typed_data = TypedData::Cast(*object_);

runtime/vm/object.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10784,6 +10784,13 @@ class Record : public Instance {
1078410784
return OFFSET_OF_RETURNED_VALUE(UntaggedRecord, data) +
1078510785
kBytesPerElement * index;
1078610786
}
10787+
static intptr_t field_index_at_offset(intptr_t offset_in_bytes) {
10788+
const intptr_t index =
10789+
(offset_in_bytes - OFFSET_OF_RETURNED_VALUE(UntaggedRecord, data)) /
10790+
kBytesPerElement;
10791+
ASSERT(index >= 0);
10792+
return index;
10793+
}
1078710794

1078810795
static intptr_t InstanceSize() {
1078910796
ASSERT(sizeof(UntaggedRecord) ==
@@ -10826,6 +10833,7 @@ class Record : public Instance {
1082610833

1082710834
FINAL_HEAP_OBJECT_IMPLEMENTATION(Record, Instance);
1082810835
friend class Class;
10836+
friend class DeferredObject; // For set_field_names.
1082910837
friend class Object;
1083010838
};
1083110839

0 commit comments

Comments
 (0)