Skip to content

Commit 9fe6571

Browse files
mralephCommit Queue
mraleph
authored and
Commit Queue
committed
[vm/compiler] Let Parameter carry its location
Previously we would specify parameter offset and base register and use SpecialParameter instruction to represent values passed in registers. This CL simplifies and merges these concepts: now every Parameter instruction simply carries around its Location. This CL also tweaks printing of stack slot locations so that they are easier to read: `S+1` becomes `fp[1]` or `sp[1]` depending on the base register, DS+1 becomes `dword fp[1]` / `dword sp[1]` and `QS+1` expands to `qword fp[1]`/`qword sp[1]`. Additionally we clean `BlockBuilder` API a bit - instead of passing `with_frame` to `AddParameter` we make this the property of the builder itself, set at construction time. Issue #54955 Closes #31956 TEST=ci Cq-Include-Trybots: luci.dart.try:vm-aot-linux-debug-simarm_x64-try,vm-aot-linux-release-arm64-try,vm-aot-linux-release-simarm_x64-try,vm-ffi-qemu-linux-release-arm-try,vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-aot-linux-product-x64-try,vm-aot-mac-release-x64-try Change-Id: Ie24dc6b6c47b3a45db722813218bc53c3a06c91d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/353081 Reviewed-by: Daco Harkes <[email protected]> Commit-Queue: Slava Egorov <[email protected]>
1 parent ac39be3 commit 9fe6571

22 files changed

+304
-432
lines changed

‎runtime/tests/vm/dart/records_return_value_unboxing_il_test.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,8 @@ void matchIL$testUnboxedRecordInTryCatch(FlowGraph graph) {
191191
]),
192192
'B2' <<
193193
match.block('CatchBlock', [
194-
'e' << match.SpecialParameter(),
195-
'st' << match.SpecialParameter(),
194+
'e' << match.Parameter(index: 2),
195+
'st' << match.Parameter(index: 3),
196196
match.MoveArgument('e'),
197197
match.StaticCall(),
198198
match.Goto('B3'),

‎runtime/vm/compiler/backend/block_builder.h

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,16 @@ namespace compiler {
1818
// Helper class for building basic blocks in SSA form.
1919
class BlockBuilder : public ValueObject {
2020
public:
21-
BlockBuilder(FlowGraph* flow_graph, BlockEntryInstr* entry)
21+
BlockBuilder(FlowGraph* flow_graph,
22+
BlockEntryInstr* entry,
23+
bool with_frame = true)
2224
: flow_graph_(flow_graph),
2325
source_(InstructionSource(flow_graph_->function().token_pos(),
2426
flow_graph->inlining_id())),
2527
entry_(entry),
2628
current_(entry),
27-
dummy_env_(new Environment(0, 0, 0, flow_graph->function(), nullptr)) {
29+
dummy_env_(new Environment(0, 0, 0, flow_graph->function(), nullptr)),
30+
with_frame_(with_frame) {
2831
// Some graph transformations use environments from block entries.
2932
entry->SetEnvironment(dummy_env_);
3033
}
@@ -69,22 +72,23 @@ class BlockBuilder : public ValueObject {
6972
return instr;
7073
}
7174

72-
Definition* AddParameter(intptr_t index, bool with_frame) {
75+
Definition* AddParameter(intptr_t index) {
7376
const auto& function = flow_graph_->function();
74-
const intptr_t param_offset = FlowGraph::ParameterOffsetAt(function, index);
77+
const auto loc = FlowGraph::ParameterLocationAt(function, index);
7578
const auto representation =
7679
FlowGraph::ParameterRepresentationAt(function, index);
77-
return AddParameter(index, param_offset, with_frame, representation);
80+
return AddParameter(index, representation,
81+
with_frame_ ? loc : loc.ToEntrySpRelative());
7882
}
7983

8084
Definition* AddParameter(intptr_t index,
81-
intptr_t param_offset,
82-
bool with_frame,
83-
Representation representation) {
85+
Representation representation,
86+
Location location = Location()) {
8487
auto normal_entry = flow_graph_->graph_entry()->normal_entry();
85-
return AddToInitialDefinitions(new ParameterInstr(
86-
/*env_index=*/index, /*param_index=*/index, param_offset, normal_entry,
87-
representation, with_frame ? FPREG : SPREG));
88+
return AddToInitialDefinitions(
89+
new ParameterInstr(normal_entry,
90+
/*env_index=*/index,
91+
/*param_index=*/index, location, representation));
8892
}
8993

9094
TokenPosition TokenPos() const { return source_.token_pos; }
@@ -162,6 +166,7 @@ class BlockBuilder : public ValueObject {
162166
BlockEntryInstr* entry_;
163167
Instruction* current_;
164168
Environment* dummy_env_;
169+
const bool with_frame_;
165170
};
166171

167172
} // namespace compiler

‎runtime/vm/compiler/backend/constant_propagator.cc

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -476,10 +476,6 @@ void ConstantPropagator::VisitAssertBoolean(AssertBooleanInstr* instr) {
476476
}
477477
}
478478

479-
void ConstantPropagator::VisitSpecialParameter(SpecialParameterInstr* instr) {
480-
SetValue(instr, non_constant_);
481-
}
482-
483479
void ConstantPropagator::VisitClosureCall(ClosureCallInstr* instr) {
484480
SetValue(instr, non_constant_);
485481
}

‎runtime/vm/compiler/backend/constant_propagator_test.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,7 @@ static void ConstantPropagatorUnboxedOpTest(
150150

151151
{
152152
BlockBuilder builder(H.flow_graph(), b1);
153-
auto v0 = builder.AddParameter(/*index=*/0, /*param_offset=*/0,
154-
/*with_frame=*/true, kTagged);
153+
auto v0 = builder.AddParameter(/*index=*/0, kTagged);
155154
builder.AddBranch(
156155
new StrictCompareInstr(
157156
InstructionSource(), Token::kEQ_STRICT, new Value(H.IntConstant(1)),

‎runtime/vm/compiler/backend/flow_graph.cc

Lines changed: 95 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,46 @@ void FlowGraph::EnsureSSATempIndex(Definition* defn, Definition* replacement) {
7272
}
7373
}
7474

75+
// When updating this check also [ParameterOffsetAt] and
76+
// [PopulateEnvironmentFromFunctionEntry]
77+
Location FlowGraph::ParameterLocationAt(const Function& function,
78+
intptr_t index) {
79+
ASSERT(index <= function.NumParameters());
80+
intptr_t offset_in_words_from_caller_sp = 0;
81+
for (intptr_t i = function.NumParameters() - 1; i > index; i--) {
82+
if (function.is_unboxed_integer_parameter_at(i)) {
83+
offset_in_words_from_caller_sp += compiler::target::kIntSpillFactor;
84+
} else if (function.is_unboxed_double_parameter_at(i)) {
85+
offset_in_words_from_caller_sp += compiler::target::kDoubleSpillFactor;
86+
} else {
87+
ASSERT(!function.is_unboxed_parameter_at(i));
88+
// Boxed parameters occupy one word.
89+
offset_in_words_from_caller_sp++;
90+
}
91+
}
92+
93+
const auto offset_in_words_from_fp =
94+
offset_in_words_from_caller_sp +
95+
compiler::target::frame_layout.param_end_from_fp + 1;
96+
97+
if (function.is_unboxed_double_parameter_at(index)) {
98+
return Location::DoubleStackSlot(offset_in_words_from_fp, FPREG);
99+
} else if (function.is_unboxed_integer_parameter_at(index)) {
100+
if (compiler::target::kIntSpillFactor == 1) {
101+
return Location::StackSlot(offset_in_words_from_fp, FPREG);
102+
} else {
103+
ASSERT(compiler::target::kIntSpillFactor == 2);
104+
return Location::Pair(
105+
Location::StackSlot(offset_in_words_from_fp, FPREG),
106+
Location::StackSlot(offset_in_words_from_fp + 1, FPREG));
107+
}
108+
} else {
109+
return Location::StackSlot(offset_in_words_from_fp, FPREG);
110+
}
111+
}
112+
113+
// When updating this check also [ParameterLocationAt] and
114+
// [PopulateEnvironmentFromFunctionEntry]
75115
intptr_t FlowGraph::ParameterOffsetAt(const Function& function,
76116
intptr_t index,
77117
bool last_slot /*=true*/) {
@@ -84,7 +124,7 @@ intptr_t FlowGraph::ParameterOffsetAt(const Function& function,
84124
param_offset += compiler::target::kDoubleSpillFactor;
85125
} else {
86126
ASSERT(!function.is_unboxed_parameter_at(i));
87-
// Unboxed parameters always occupy one word
127+
// Boxed parameters occupy one word.
88128
param_offset++;
89129
}
90130
}
@@ -1203,6 +1243,8 @@ void FlowGraph::Rename(GrowableArray<PhiInstr*>* live_phis,
12031243
#endif // defined(DEBUG)
12041244
}
12051245

1246+
// When updating this check also [ParameterLocationAt] and
1247+
// [ParameterOffsetAt].
12061248
void FlowGraph::PopulateEnvironmentFromFunctionEntry(
12071249
FunctionEntryInstr* function_entry,
12081250
GrowableArray<Definition*>* env,
@@ -1218,32 +1260,44 @@ void FlowGraph::PopulateEnvironmentFromFunctionEntry(
12181260

12191261
ASSERT(variable_count() == env->length());
12201262
ASSERT(direct_parameter_count <= env->length());
1221-
intptr_t param_offset = 0;
1263+
1264+
intptr_t offset_in_words_from_fp =
1265+
(compiler::target::frame_layout.param_end_from_fp + 1) +
1266+
direct_parameters_size_;
12221267
for (intptr_t i = 0; i < direct_parameter_count; i++) {
12231268
ASSERT(FLAG_precompiled_mode || !function().is_unboxed_parameter_at(i));
1224-
ParameterInstr* param;
12251269

12261270
const intptr_t index = (function().IsFactory() ? (i - 1) : i);
12271271

1272+
Representation rep;
1273+
Location location;
1274+
12281275
if (index >= 0 && function().is_unboxed_integer_parameter_at(index)) {
1229-
constexpr intptr_t kCorrection = compiler::target::kIntSpillFactor - 1;
1230-
param = new (zone()) ParameterInstr(/*env_index=*/i, /*param_index=*/i,
1231-
param_offset + kCorrection,
1232-
function_entry, kUnboxedInt64);
1233-
param_offset += compiler::target::kIntSpillFactor;
1276+
rep = kUnboxedInt64;
1277+
offset_in_words_from_fp -= compiler::target::kIntSpillFactor;
1278+
if (compiler::target::kIntSpillFactor == 1) {
1279+
location = Location::StackSlot(offset_in_words_from_fp, FPREG);
1280+
} else {
1281+
ASSERT(compiler::target::kIntSpillFactor == 2);
1282+
location = Location::Pair(
1283+
Location::StackSlot(offset_in_words_from_fp, FPREG),
1284+
Location::StackSlot(offset_in_words_from_fp + 1, FPREG));
1285+
}
12341286
} else if (index >= 0 && function().is_unboxed_double_parameter_at(index)) {
1235-
constexpr intptr_t kCorrection = compiler::target::kDoubleSpillFactor - 1;
1236-
param = new (zone()) ParameterInstr(/*env_index=*/i, /*param_index=*/i,
1237-
param_offset + kCorrection,
1238-
function_entry, kUnboxedDouble);
1239-
param_offset += compiler::target::kDoubleSpillFactor;
1287+
rep = kUnboxedDouble;
1288+
offset_in_words_from_fp -= compiler::target::kDoubleSpillFactor;
1289+
location = Location::DoubleStackSlot(offset_in_words_from_fp, FPREG);
12401290
} else {
12411291
ASSERT(index < 0 || !function().is_unboxed_parameter_at(index));
1242-
param =
1243-
new (zone()) ParameterInstr(/*env_index=*/i, /*param_index=*/i,
1244-
param_offset, function_entry, kTagged);
1245-
param_offset++;
1292+
rep = kTagged;
1293+
offset_in_words_from_fp -= 1;
1294+
location = Location::StackSlot(offset_in_words_from_fp, FPREG);
12461295
}
1296+
1297+
auto param = new (zone()) ParameterInstr(function_entry,
1298+
/*env_index=*/i,
1299+
/*param_index=*/i, location, rep);
1300+
12471301
AllocateSSAIndex(param);
12481302
AddToInitialDefinitions(function_entry, param);
12491303
(*env)[i] = param;
@@ -1296,16 +1350,25 @@ void FlowGraph::PopulateEnvironmentFromFunctionEntry(
12961350

12971351
// Replace the argument descriptor slot with a special parameter.
12981352
if (parsed_function().has_arg_desc_var()) {
1299-
Definition* defn =
1300-
new (Z) SpecialParameterInstr(SpecialParameterInstr::kArgDescriptor,
1301-
DeoptId::kNone, function_entry);
1353+
auto defn = new (Z)
1354+
ParameterInstr(function_entry, ArgumentDescriptorEnvIndex(),
1355+
ParameterInstr::kNotFunctionParameter,
1356+
Location::RegisterLocation(ARGS_DESC_REG), kTagged);
13021357
AllocateSSAIndex(defn);
13031358
AddToInitialDefinitions(function_entry, defn);
13041359
(*env)[ArgumentDescriptorEnvIndex()] = defn;
13051360
}
13061361
}
13071362
}
13081363

1364+
static Location EnvIndexToStackLocation(intptr_t num_direct_parameters,
1365+
intptr_t env_index) {
1366+
return Location::StackSlot(
1367+
compiler::target::frame_layout.FrameSlotForVariableIndex(
1368+
num_direct_parameters - env_index),
1369+
FPREG);
1370+
}
1371+
13091372
void FlowGraph::PopulateEnvironmentFromOsrEntry(
13101373
OsrEntryInstr* osr_entry,
13111374
GrowableArray<Definition*>* env) {
@@ -1319,8 +1382,9 @@ void FlowGraph::PopulateEnvironmentFromOsrEntry(
13191382
const intptr_t param_index = (i < num_direct_parameters())
13201383
? i
13211384
: ParameterInstr::kNotFunctionParameter;
1322-
ParameterInstr* param = new (zone())
1323-
ParameterInstr(/*env_index=*/i, param_index, i, osr_entry, kTagged);
1385+
ParameterInstr* param = new (zone()) ParameterInstr(
1386+
osr_entry, /*env_index=*/i, param_index,
1387+
EnvIndexToStackLocation(num_direct_parameters(), i), kTagged);
13241388
AllocateSSAIndex(param);
13251389
AddToInitialDefinitions(osr_entry, param);
13261390
(*env)[i] = param;
@@ -1342,21 +1406,19 @@ void FlowGraph::PopulateEnvironmentFromCatchEntry(
13421406
// Add real definitions for all locals and parameters.
13431407
ASSERT(variable_count() == env->length());
13441408
for (intptr_t i = 0, n = variable_count(); i < n; ++i) {
1345-
// Replace usages of the raw exception/stacktrace variables with
1346-
// [SpecialParameterInstr]s.
1347-
Definition* param = nullptr;
1409+
// Local variables will arive on the stack while exception and
1410+
// stack trace will be passed in fixed registers.
1411+
Location loc;
13481412
if (raw_exception_var_envindex == i) {
1349-
param = new (Z) SpecialParameterInstr(SpecialParameterInstr::kException,
1350-
DeoptId::kNone, catch_entry);
1413+
loc = LocationExceptionLocation();
13511414
} else if (raw_stacktrace_var_envindex == i) {
1352-
param = new (Z) SpecialParameterInstr(SpecialParameterInstr::kStackTrace,
1353-
DeoptId::kNone, catch_entry);
1415+
loc = LocationStackTraceLocation();
13541416
} else {
1355-
param = new (Z)
1356-
ParameterInstr(/*env_index=*/i,
1357-
/*param_index=*/ParameterInstr::kNotFunctionParameter,
1358-
i, catch_entry, kTagged);
1417+
loc = EnvIndexToStackLocation(num_direct_parameters(), i);
13591418
}
1419+
auto param = new (Z) ParameterInstr(
1420+
catch_entry, /*env_index=*/i,
1421+
/*param_index=*/ParameterInstr::kNotFunctionParameter, loc, kTagged);
13601422

13611423
AllocateSSAIndex(param); // New SSA temp.
13621424
(*env)[i] = param;

‎runtime/vm/compiler/backend/flow_graph.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ class FlowGraph : public ZoneAllocated {
151151
static intptr_t ParameterOffsetAt(const Function& function,
152152
intptr_t index,
153153
bool last_slot = true);
154+
static Location ParameterLocationAt(const Function& function, intptr_t index);
154155

155156
static Representation ParameterRepresentationAt(const Function& function,
156157
intptr_t index);

‎runtime/vm/compiler/backend/flow_graph_checker.cc

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -181,9 +181,7 @@ void FlowGraphChecker::VisitInstructions(BlockEntryInstr* block) {
181181
if (auto entry = block->AsBlockEntryWithInitialDefs()) {
182182
for (auto def : *entry->initial_definitions()) {
183183
ASSERT(def != nullptr);
184-
ASSERT1(
185-
def->IsConstant() || def->IsParameter() || def->IsSpecialParameter(),
186-
def);
184+
ASSERT1(def->IsConstant() || def->IsParameter(), def);
187185
// Make sure block lookup agrees.
188186
ASSERT1(def->GetBlock() == entry, def);
189187
// Initial definitions are partially linked into graph.
@@ -360,8 +358,7 @@ void FlowGraphChecker::VisitUseDef(Instruction* instruction,
360358
// Phis are never linked into graph.
361359
ASSERT1(def->next() == nullptr, def);
362360
ASSERT1(def->previous() == nullptr, def);
363-
} else if (def->IsConstant() || def->IsParameter() ||
364-
def->IsSpecialParameter()) {
361+
} else if (def->IsConstant() || def->IsParameter()) {
365362
// Initial definitions are partially linked into graph, but some
366363
// constants are fully linked into graph (so no next() assert).
367364
ASSERT1(def->previous() != nullptr, def);

‎runtime/vm/compiler/backend/flow_graph_test.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ ISOLATE_UNIT_TEST_CASE(FlowGraph_UnboxInt64Phi) {
3838

3939
{
4040
BlockBuilder builder(H.flow_graph(), normal_entry);
41-
v0 = builder.AddParameter(0, 0, /*with_frame=*/true, kTagged);
41+
v0 = builder.AddParameter(0, kTagged);
4242
builder.AddInstruction(new GotoInstr(loop_header, S.GetNextDeoptId()));
4343
}
4444

‎runtime/vm/compiler/backend/il.cc

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4917,42 +4917,6 @@ void MaterializeObjectInstr::RemapRegisters(intptr_t* cpu_reg_slots,
49174917
}
49184918
}
49194919

4920-
const char* SpecialParameterInstr::KindToCString(SpecialParameterKind k) {
4921-
switch (k) {
4922-
#define KIND_CASE(Name) \
4923-
case SpecialParameterKind::k##Name: \
4924-
return #Name;
4925-
FOR_EACH_SPECIAL_PARAMETER_KIND(KIND_CASE)
4926-
#undef KIND_CASE
4927-
}
4928-
return nullptr;
4929-
}
4930-
4931-
bool SpecialParameterInstr::ParseKind(const char* str,
4932-
SpecialParameterKind* out) {
4933-
ASSERT(str != nullptr && out != nullptr);
4934-
#define KIND_CASE(Name) \
4935-
if (strcmp(str, #Name) == 0) { \
4936-
*out = SpecialParameterKind::k##Name; \
4937-
return true; \
4938-
}
4939-
FOR_EACH_SPECIAL_PARAMETER_KIND(KIND_CASE)
4940-
#undef KIND_CASE
4941-
return false;
4942-
}
4943-
4944-
LocationSummary* SpecialParameterInstr::MakeLocationSummary(Zone* zone,
4945-
bool opt) const {
4946-
// Only appears in initial definitions, never in normal code.
4947-
UNREACHABLE();
4948-
return nullptr;
4949-
}
4950-
4951-
void SpecialParameterInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
4952-
// Only appears in initial definitions, never in normal code.
4953-
UNREACHABLE();
4954-
}
4955-
49564920
LocationSummary* MakeTempInstr::MakeLocationSummary(Zone* zone,
49574921
bool optimizing) const {
49584922
ASSERT(!optimizing);

0 commit comments

Comments
 (0)