Skip to content

Commit 61a944c

Browse files
mkustermanncommit-bot@chromium.org
mkustermann
authored and
[email protected]
committed
[vm/compiler] Align BoxInt64 implementation on x64 with arm/arm64 by calling a shared slow path
Change-Id: I3dcdc922e00898e49bf00b985cb2af747005d35f Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/142623 Commit-Queue: Martin Kustermann <[email protected]> Reviewed-by: Alexander Markov <[email protected]>
1 parent cb6ed67 commit 61a944c

File tree

9 files changed

+186
-77
lines changed

9 files changed

+186
-77
lines changed

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

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4960,24 +4960,35 @@ LocationSummary* BoxInt64Instr::MakeLocationSummary(Zone* zone,
49604960
// Shared slow path is used in BoxInt64Instr::EmitNativeCode in
49614961
// FLAG_use_bare_instructions mode and only after VM isolate stubs where
49624962
// replaced with isolate-specific stubs.
4963+
auto object_store = Isolate::Current()->object_store();
4964+
const bool stubs_in_vm_isolate =
4965+
object_store->allocate_mint_with_fpu_regs_stub()
4966+
->ptr()
4967+
->InVMIsolateHeap() ||
4968+
object_store->allocate_mint_without_fpu_regs_stub()
4969+
->ptr()
4970+
->InVMIsolateHeap();
4971+
const bool shared_slow_path_call = SlowPathSharingSupported(opt) &&
4972+
FLAG_use_bare_instructions &&
4973+
!stubs_in_vm_isolate;
49634974
LocationSummary* summary = new (zone) LocationSummary(
49644975
zone, kNumInputs, kNumTemps,
49654976
ValueFitsSmi()
49664977
? LocationSummary::kNoCall
4967-
: ((SlowPathSharingSupported(opt) && FLAG_use_bare_instructions &&
4968-
!Isolate::Current()
4969-
->object_store()
4970-
->allocate_mint_with_fpu_regs_stub()
4971-
->ptr()
4972-
->InVMIsolateHeap())
4973-
? LocationSummary::kCallOnSharedSlowPath
4974-
: LocationSummary::kCallOnSlowPath));
4978+
: ((shared_slow_path_call ? LocationSummary::kCallOnSharedSlowPath
4979+
: LocationSummary::kCallOnSlowPath)));
49754980
summary->set_in(0, Location::Pair(Location::RequiresRegister(),
49764981
Location::RequiresRegister()));
4977-
if (!ValueFitsSmi()) {
4978-
summary->set_temp(0, Location::RegisterLocation(R1));
4982+
if (ValueFitsSmi()) {
4983+
summary->set_out(0, Location::RequiresRegister());
4984+
} else if (shared_slow_path_call) {
4985+
summary->set_out(0,
4986+
Location::RegisterLocation(AllocateMintABI::kResultReg));
4987+
summary->set_temp(0, Location::RegisterLocation(AllocateMintABI::kTempReg));
4988+
} else {
4989+
summary->set_out(0, Location::RequiresRegister());
4990+
summary->set_temp(0, Location::RequiresRegister());
49794991
}
4980-
summary->set_out(0, Location::RegisterLocation(R0));
49814992
return summary;
49824993
}
49834994

@@ -5013,7 +5024,8 @@ void BoxInt64Instr::EmitNativeCode(FlowGraphCompiler* compiler) {
50135024
live_fpu_regs ? object_store->allocate_mint_with_fpu_regs_stub()
50145025
: object_store->allocate_mint_without_fpu_regs_stub());
50155026

5016-
ASSERT(!locs()->live_registers()->ContainsRegister(R0));
5027+
ASSERT(!locs()->live_registers()->ContainsRegister(
5028+
AllocateMintABI::kResultReg));
50175029
auto extended_env = compiler->SlowPathEnvironmentFor(this, 0);
50185030
compiler->GenerateStubCall(token_pos(), stub, PcDescriptorsLayout::kOther,
50195031
locs(), DeoptId::kNone, extended_env);

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

Lines changed: 26 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -4168,16 +4168,14 @@ LocationSummary* BoxInt64Instr::MakeLocationSummary(Zone* zone,
41684168
// Shared slow path is used in BoxInt64Instr::EmitNativeCode in
41694169
// FLAG_use_bare_instructions mode and only after VM isolate stubs where
41704170
// replaced with isolate-specific stubs.
4171-
const bool stubs_in_vm_isolate = (Isolate::Current()
4172-
->object_store()
4173-
->allocate_mint_with_fpu_regs_stub()
4174-
->ptr()
4175-
->InVMIsolateHeap() ||
4176-
Isolate::Current()
4177-
->object_store()
4178-
->allocate_mint_without_fpu_regs_stub()
4179-
->ptr()
4180-
->InVMIsolateHeap());
4171+
auto object_store = Isolate::Current()->object_store();
4172+
const bool stubs_in_vm_isolate =
4173+
object_store->allocate_mint_with_fpu_regs_stub()
4174+
->ptr()
4175+
->InVMIsolateHeap() ||
4176+
object_store->allocate_mint_without_fpu_regs_stub()
4177+
->ptr()
4178+
->InVMIsolateHeap();
41814179
const bool shared_slow_path_call = SlowPathSharingSupported(opt) &&
41824180
FLAG_use_bare_instructions &&
41834181
!stubs_in_vm_isolate;
@@ -4187,13 +4185,13 @@ LocationSummary* BoxInt64Instr::MakeLocationSummary(Zone* zone,
41874185
? LocationSummary::kNoCall
41884186
: shared_slow_path_call ? LocationSummary::kCallOnSharedSlowPath
41894187
: LocationSummary::kCallOnSlowPath);
4190-
41914188
summary->set_in(0, Location::RequiresRegister());
41924189
if (ValueFitsSmi()) {
41934190
summary->set_out(0, Location::RequiresRegister());
41944191
} else if (shared_slow_path_call) {
4195-
summary->set_out(0, Location::RegisterLocation(R0));
4196-
summary->set_temp(0, Location::RegisterLocation(R1));
4192+
summary->set_out(0,
4193+
Location::RegisterLocation(AllocateMintABI::kResultReg));
4194+
summary->set_temp(0, Location::RegisterLocation(AllocateMintABI::kTempReg));
41974195
} else {
41984196
summary->set_out(0, Location::RequiresRegister());
41994197
summary->set_temp(0, Location::RequiresRegister());
@@ -4216,28 +4214,25 @@ void BoxInt64Instr::EmitNativeCode(FlowGraphCompiler* compiler) {
42164214
__ b(&done, NO_OVERFLOW);
42174215

42184216
Register temp = locs()->temp(0).reg();
4219-
42204217
if (compiler->intrinsic_mode()) {
42214218
__ TryAllocate(compiler->mint_class(),
42224219
compiler->intrinsic_slow_path_label(), out, temp);
4220+
} else if (locs()->call_on_shared_slow_path()) {
4221+
auto object_store = compiler->isolate()->object_store();
4222+
const bool live_fpu_regs = locs()->live_registers()->FpuRegisterCount() > 0;
4223+
const auto& stub = Code::ZoneHandle(
4224+
compiler->zone(),
4225+
live_fpu_regs ? object_store->allocate_mint_with_fpu_regs_stub()
4226+
: object_store->allocate_mint_without_fpu_regs_stub());
4227+
4228+
ASSERT(!locs()->live_registers()->ContainsRegister(
4229+
AllocateMintABI::kResultReg));
4230+
auto extended_env = compiler->SlowPathEnvironmentFor(this, 0);
4231+
compiler->GenerateStubCall(token_pos(), stub, PcDescriptorsLayout::kOther,
4232+
locs(), DeoptId::kNone, extended_env);
42234233
} else {
4224-
if (locs()->call_on_shared_slow_path()) {
4225-
auto object_store = compiler->isolate()->object_store();
4226-
const bool live_fpu_regs =
4227-
locs()->live_registers()->FpuRegisterCount() > 0;
4228-
const auto& stub = Code::ZoneHandle(
4229-
compiler->zone(),
4230-
live_fpu_regs ? object_store->allocate_mint_with_fpu_regs_stub()
4231-
: object_store->allocate_mint_without_fpu_regs_stub());
4232-
4233-
ASSERT(!locs()->live_registers()->ContainsRegister(R0));
4234-
auto extended_env = compiler->SlowPathEnvironmentFor(this, 0);
4235-
compiler->GenerateStubCall(token_pos(), stub, PcDescriptorsLayout::kOther,
4236-
locs(), DeoptId::kNone, extended_env);
4237-
} else {
4238-
BoxAllocationSlowPath::Allocate(compiler, this, compiler->mint_class(),
4239-
out, temp);
4240-
}
4234+
BoxAllocationSlowPath::Allocate(compiler, this, compiler->mint_class(), out,
4235+
temp);
42414236
}
42424237

42434238
__ StoreToOffset(in, out, Mint::value_offset() - kHeapObjectTag);

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

Lines changed: 58 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4446,15 +4446,37 @@ LocationSummary* BoxInt64Instr::MakeLocationSummary(Zone* zone,
44464446
bool opt) const {
44474447
const intptr_t kNumInputs = 1;
44484448
const intptr_t kNumTemps = ValueFitsSmi() ? 0 : 1;
4449-
LocationSummary* summary = new (zone)
4450-
LocationSummary(zone, kNumInputs, kNumTemps,
4451-
ValueFitsSmi() ? LocationSummary::kNoCall
4452-
: LocationSummary::kCallOnSlowPath);
4449+
// Shared slow path is used in BoxInt64Instr::EmitNativeCode in
4450+
// FLAG_use_bare_instructions mode and only after VM isolate stubs where
4451+
// replaced with isolate-specific stubs.
4452+
auto object_store = Isolate::Current()->object_store();
4453+
const bool stubs_in_vm_isolate =
4454+
object_store->allocate_mint_with_fpu_regs_stub()
4455+
->ptr()
4456+
->InVMIsolateHeap() ||
4457+
object_store->allocate_mint_without_fpu_regs_stub()
4458+
->ptr()
4459+
->InVMIsolateHeap();
4460+
const bool shared_slow_path_call = SlowPathSharingSupported(opt) &&
4461+
FLAG_use_bare_instructions &&
4462+
!stubs_in_vm_isolate;
4463+
LocationSummary* summary = new (zone) LocationSummary(
4464+
zone, kNumInputs, kNumTemps,
4465+
ValueFitsSmi()
4466+
? LocationSummary::kNoCall
4467+
: ((shared_slow_path_call ? LocationSummary::kCallOnSharedSlowPath
4468+
: LocationSummary::kCallOnSlowPath)));
44534469
summary->set_in(0, Location::RequiresRegister());
4454-
if (!ValueFitsSmi()) {
4470+
if (ValueFitsSmi()) {
4471+
summary->set_out(0, Location::RequiresRegister());
4472+
} else if (shared_slow_path_call) {
4473+
summary->set_out(0,
4474+
Location::RegisterLocation(AllocateMintABI::kResultReg));
4475+
summary->set_temp(0, Location::RegisterLocation(AllocateMintABI::kTempReg));
4476+
} else {
4477+
summary->set_out(0, Location::RequiresRegister());
44554478
summary->set_temp(0, Location::RequiresRegister());
44564479
}
4457-
summary->set_out(0, Location::RequiresRegister());
44584480
return summary;
44594481
}
44604482

@@ -4463,17 +4485,39 @@ void BoxInt64Instr::EmitNativeCode(FlowGraphCompiler* compiler) {
44634485
const Register value = locs()->in(0).reg();
44644486
__ MoveRegister(out, value);
44654487
__ SmiTag(out);
4466-
if (!ValueFitsSmi()) {
4467-
const Register temp = locs()->temp(0).reg();
4468-
compiler::Label done;
4469-
// If the value doesn't fit in a smi, the tagging changes the sign,
4470-
// which causes the overflow flag to be set.
4471-
__ j(NO_OVERFLOW, &done);
4488+
if (ValueFitsSmi()) {
4489+
return;
4490+
}
4491+
// If the value doesn't fit in a smi, the tagging changes the sign,
4492+
// which causes the overflow flag to be set.
4493+
compiler::Label done;
4494+
__ j(NO_OVERFLOW, &done);
4495+
4496+
const Register temp = locs()->temp(0).reg();
4497+
if (compiler->intrinsic_mode()) {
4498+
__ TryAllocate(compiler->mint_class(),
4499+
compiler->intrinsic_slow_path_label(),
4500+
/*near_jump=*/true, out, temp);
4501+
} else if (locs()->call_on_shared_slow_path()) {
4502+
auto object_store = compiler->isolate()->object_store();
4503+
const bool live_fpu_regs = locs()->live_registers()->FpuRegisterCount() > 0;
4504+
const auto& stub = Code::ZoneHandle(
4505+
compiler->zone(),
4506+
live_fpu_regs ? object_store->allocate_mint_with_fpu_regs_stub()
4507+
: object_store->allocate_mint_without_fpu_regs_stub());
4508+
4509+
ASSERT(!locs()->live_registers()->ContainsRegister(
4510+
AllocateMintABI::kResultReg));
4511+
auto extended_env = compiler->SlowPathEnvironmentFor(this, 0);
4512+
compiler->GenerateStubCall(token_pos(), stub, PcDescriptorsLayout::kOther,
4513+
locs(), DeoptId::kNone, extended_env);
4514+
} else {
44724515
BoxAllocationSlowPath::Allocate(compiler, this, compiler->mint_class(), out,
44734516
temp);
4474-
__ movq(compiler::FieldAddress(out, Mint::value_offset()), value);
4475-
__ Bind(&done);
44764517
}
4518+
4519+
__ movq(compiler::FieldAddress(out, Mint::value_offset()), value);
4520+
__ Bind(&done);
44774521
}
44784522

44794523
LocationSummary* BinaryDoubleOpInstr::MakeLocationSummary(Zone* zone,

‎runtime/vm/compiler/stub_code_compiler_arm.cc‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ void GenerateSharedStubGeneric(Assembler* assembler,
195195
}
196196
__ LeaveStubFrame();
197197
__ PopRegisters(all_registers);
198-
__ Pop(LR);
198+
__ Drop(1); // We use the LR restored via LeaveStubFrame.
199199
__ bx(LR);
200200
}
201201

@@ -208,7 +208,7 @@ static void GenerateSharedStub(Assembler* assembler,
208208
ASSERT(!store_runtime_result_in_r0 || allow_return);
209209
auto perform_runtime_call = [&]() {
210210
if (store_runtime_result_in_r0) {
211-
__ PushRegister(LR); // Push an even register.
211+
__ PushRegister(LR);
212212
}
213213
__ CallRuntime(*target, /*argument_count=*/0);
214214
if (store_runtime_result_in_r0) {
@@ -1205,12 +1205,13 @@ void StubCodeCompiler::GenerateAllocateMintSharedWithFPURegsStub(
12051205
// For test purpose call allocation stub without inline allocation attempt.
12061206
if (!FLAG_use_slow_path) {
12071207
Label slow_case;
1208-
__ TryAllocate(compiler::MintClass(), &slow_case, /*instance_reg=*/R0,
1209-
/*temp_reg=*/R1);
1208+
__ TryAllocate(compiler::MintClass(), &slow_case,
1209+
AllocateMintABI::kResultReg, AllocateMintABI::kTempReg);
12101210
__ Ret();
12111211

12121212
__ Bind(&slow_case);
12131213
}
1214+
COMPILE_ASSERT(AllocateMintABI::kResultReg == R0);
12141215
GenerateSharedStub(assembler, /*save_fpu_registers=*/true,
12151216
&kAllocateMintRuntimeEntry,
12161217
target::Thread::allocate_mint_with_fpu_regs_stub_offset(),
@@ -1224,12 +1225,13 @@ void StubCodeCompiler::GenerateAllocateMintSharedWithoutFPURegsStub(
12241225
// For test purpose call allocation stub without inline allocation attempt.
12251226
if (!FLAG_use_slow_path) {
12261227
Label slow_case;
1227-
__ TryAllocate(compiler::MintClass(), &slow_case, /*instance_reg=*/R0,
1228-
/*temp_reg=*/R1);
1228+
__ TryAllocate(compiler::MintClass(), &slow_case,
1229+
AllocateMintABI::kResultReg, AllocateMintABI::kTempReg);
12291230
__ Ret();
12301231

12311232
__ Bind(&slow_case);
12321233
}
1234+
COMPILE_ASSERT(AllocateMintABI::kResultReg == R0);
12331235
GenerateSharedStub(
12341236
assembler, /*save_fpu_registers=*/false, &kAllocateMintRuntimeEntry,
12351237
target::Thread::allocate_mint_without_fpu_regs_stub_offset(),

‎runtime/vm/compiler/stub_code_compiler_arm64.cc‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ static void GenerateSharedStubGeneric(
209209
}
210210
__ LeaveStubFrame();
211211
__ PopRegisters(all_registers);
212-
__ Pop(LR);
212+
__ Drop(1); // We use the LR restored via LeaveStubFrame.
213213
__ ret(LR);
214214
}
215215

@@ -1331,12 +1331,13 @@ void StubCodeCompiler::GenerateAllocateMintSharedWithFPURegsStub(
13311331
// For test purpose call allocation stub without inline allocation attempt.
13321332
if (!FLAG_use_slow_path) {
13331333
Label slow_case;
1334-
__ TryAllocate(compiler::MintClass(), &slow_case, /*instance_reg=*/R0,
1335-
/*temp_reg=*/R1);
1334+
__ TryAllocate(compiler::MintClass(), &slow_case,
1335+
AllocateMintABI::kResultReg, AllocateMintABI::kTempReg);
13361336
__ Ret();
13371337

13381338
__ Bind(&slow_case);
13391339
}
1340+
COMPILE_ASSERT(AllocateMintABI::kResultReg == R0);
13401341
GenerateSharedStub(assembler, /*save_fpu_registers=*/true,
13411342
&kAllocateMintRuntimeEntry,
13421343
target::Thread::allocate_mint_with_fpu_regs_stub_offset(),
@@ -1349,12 +1350,13 @@ void StubCodeCompiler::GenerateAllocateMintSharedWithoutFPURegsStub(
13491350
// For test purpose call allocation stub without inline allocation attempt.
13501351
if (!FLAG_use_slow_path) {
13511352
Label slow_case;
1352-
__ TryAllocate(compiler::MintClass(), &slow_case, /*instance_reg=*/R0,
1353-
/*temp_reg=*/R1);
1353+
__ TryAllocate(compiler::MintClass(), &slow_case,
1354+
AllocateMintABI::kResultReg, AllocateMintABI::kTempReg);
13541355
__ Ret();
13551356

13561357
__ Bind(&slow_case);
13571358
}
1359+
COMPILE_ASSERT(AllocateMintABI::kResultReg == R0);
13581360
GenerateSharedStub(
13591361
assembler, /*save_fpu_registers=*/false, &kAllocateMintRuntimeEntry,
13601362
target::Thread::allocate_mint_without_fpu_regs_stub_offset(),

0 commit comments

Comments
 (0)