Skip to content

Commit 4605011

Browse files
Artem Serovcommit-bot@chromium.org
Artem Serov
authored and
commit-bot@chromium.org
committed
[vm/compiler] Arm64: Improve StoreIndexedInstr.
Reuse offset addressing mode with LoadIndexedInstr; this will save 1 instruction generated for the case: add x0, x1, x2, LSL #SCALE ldr x3, [x0, #OFFSET] vs add x0, x1, #OFFSET add x0, x0, x2, LSL #SCALE ldr x3, [x0] Change-Id: I35d9e9553f312529d1c71fdd4475545e3c439179 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/121992 Commit-Queue: Vyacheslav Egorov <vegorov@google.com> Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
1 parent 3d67e7d commit 4605011

File tree

4 files changed

+104
-95
lines changed

4 files changed

+104
-95
lines changed

‎runtime/vm/compiler/assembler/assembler_arm64.cc‎

Lines changed: 30 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1742,80 +1742,72 @@ Address Assembler::ElementAddressForIntIndex(bool is_external,
17421742
Register array,
17431743
intptr_t index) const {
17441744
const int64_t offset =
1745-
index * index_scale +
1746-
(is_external ? 0
1747-
: (target::Instance::DataOffsetFor(cid) - kHeapObjectTag));
1745+
index * index_scale + HeapDataOffset(is_external, cid);
17481746
ASSERT(Utils::IsInt(32, offset));
17491747
const OperandSize size = Address::OperandSizeFor(cid);
17501748
ASSERT(Address::CanHoldOffset(offset, Address::Offset, size));
17511749
return Address(array, static_cast<int32_t>(offset), Address::Offset, size);
17521750
}
17531751

1754-
void Assembler::LoadElementAddressForIntIndex(Register address,
1755-
bool is_external,
1756-
intptr_t cid,
1757-
intptr_t index_scale,
1758-
Register array,
1759-
intptr_t index) {
1752+
void Assembler::ComputeElementAddressForIntIndex(Register address,
1753+
bool is_external,
1754+
intptr_t cid,
1755+
intptr_t index_scale,
1756+
Register array,
1757+
intptr_t index) {
17601758
const int64_t offset =
1761-
index * index_scale +
1762-
(is_external ? 0
1763-
: (target::Instance::DataOffsetFor(cid) - kHeapObjectTag));
1759+
index * index_scale + HeapDataOffset(is_external, cid);
17641760
AddImmediate(address, array, offset);
17651761
}
17661762

1767-
Address Assembler::ElementAddressForRegIndex(bool is_load,
1768-
bool is_external,
1763+
Address Assembler::ElementAddressForRegIndex(bool is_external,
17691764
intptr_t cid,
17701765
intptr_t index_scale,
17711766
Register array,
1772-
Register index) {
1773-
return ElementAddressForRegIndexWithSize(is_load,
1774-
is_external,
1767+
Register index,
1768+
Register temp) {
1769+
return ElementAddressForRegIndexWithSize(is_external,
17751770
cid,
17761771
Address::OperandSizeFor(cid),
17771772
index_scale,
17781773
array,
1779-
index);
1774+
index,
1775+
temp);
17801776
}
17811777

1782-
Address Assembler::ElementAddressForRegIndexWithSize(bool is_load,
1783-
bool is_external,
1778+
Address Assembler::ElementAddressForRegIndexWithSize(bool is_external,
17841779
intptr_t cid,
17851780
OperandSize size,
17861781
intptr_t index_scale,
17871782
Register array,
1788-
Register index) {
1783+
Register index,
1784+
Register temp) {
17891785
// Note that index is expected smi-tagged, (i.e, LSL 1) for all arrays.
17901786
const intptr_t shift = Utils::ShiftForPowerOfTwo(index_scale) - kSmiTagShift;
1791-
const int32_t offset =
1792-
is_external ? 0 : (target::Instance::DataOffsetFor(cid) - kHeapObjectTag);
1793-
ASSERT(array != TMP);
1794-
ASSERT(index != TMP);
1795-
const Register base = is_load ? TMP : index;
1787+
const int32_t offset = HeapDataOffset(is_external, cid);
1788+
ASSERT(array != temp);
1789+
ASSERT(index != temp);
17961790
if ((offset == 0) && (shift == 0)) {
17971791
return Address(array, index, UXTX, Address::Unscaled);
17981792
} else if (shift < 0) {
17991793
ASSERT(shift == -1);
1800-
add(base, array, Operand(index, ASR, 1));
1794+
add(temp, array, Operand(index, ASR, 1));
18011795
} else {
1802-
add(base, array, Operand(index, LSL, shift));
1796+
add(temp, array, Operand(index, LSL, shift));
18031797
}
18041798
ASSERT(Address::CanHoldOffset(offset, Address::Offset, size));
1805-
return Address(base, offset, Address::Offset, size);
1799+
return Address(temp, offset, Address::Offset, size);
18061800
}
18071801

1808-
void Assembler::LoadElementAddressForRegIndex(Register address,
1809-
bool is_load,
1810-
bool is_external,
1811-
intptr_t cid,
1812-
intptr_t index_scale,
1813-
Register array,
1814-
Register index) {
1802+
void Assembler::ComputeElementAddressForRegIndex(Register address,
1803+
bool is_external,
1804+
intptr_t cid,
1805+
intptr_t index_scale,
1806+
Register array,
1807+
Register index) {
18151808
// Note that index is expected smi-tagged, (i.e, LSL 1) for all arrays.
18161809
const intptr_t shift = Utils::ShiftForPowerOfTwo(index_scale) - kSmiTagShift;
1817-
const int32_t offset =
1818-
is_external ? 0 : (target::Instance::DataOffsetFor(cid) - kHeapObjectTag);
1810+
const int32_t offset = HeapDataOffset(is_external, cid);
18191811
if (shift == 0) {
18201812
add(address, array, Operand(index));
18211813
} else if (shift < 0) {

‎runtime/vm/compiler/assembler/assembler_arm64.h‎

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1623,37 +1623,44 @@ class Assembler : public AssemblerBase {
16231623
intptr_t index_scale,
16241624
Register array,
16251625
intptr_t index) const;
1626-
void LoadElementAddressForIntIndex(Register address,
1627-
bool is_external,
1628-
intptr_t cid,
1629-
intptr_t index_scale,
1630-
Register array,
1631-
intptr_t index);
1632-
Address ElementAddressForRegIndex(bool is_load,
1633-
bool is_external,
1626+
void ComputeElementAddressForIntIndex(Register address,
1627+
bool is_external,
1628+
intptr_t cid,
1629+
intptr_t index_scale,
1630+
Register array,
1631+
intptr_t index);
1632+
Address ElementAddressForRegIndex(bool is_external,
16341633
intptr_t cid,
16351634
intptr_t index_scale,
16361635
Register array,
1637-
Register index);
1636+
Register index,
1637+
Register temp);
16381638

16391639
// Special version of ElementAddressForRegIndex for the case when cid and
16401640
// operand size for the target load don't match (e.g. when loading a few
16411641
// elements of the array with one load).
1642-
Address ElementAddressForRegIndexWithSize(bool is_load,
1643-
bool is_external,
1642+
Address ElementAddressForRegIndexWithSize(bool is_external,
16441643
intptr_t cid,
16451644
OperandSize size,
16461645
intptr_t index_scale,
16471646
Register array,
1648-
Register index);
1649-
1650-
void LoadElementAddressForRegIndex(Register address,
1651-
bool is_load,
1652-
bool is_external,
1653-
intptr_t cid,
1654-
intptr_t index_scale,
1655-
Register array,
1656-
Register index);
1647+
Register index,
1648+
Register temp);
1649+
1650+
void ComputeElementAddressForRegIndex(Register address,
1651+
bool is_external,
1652+
intptr_t cid,
1653+
intptr_t index_scale,
1654+
Register array,
1655+
Register index);
1656+
1657+
// Returns object data offset for address calculation; for heap objects also
1658+
// accounts for the tag.
1659+
static int32_t HeapDataOffset(bool is_external, intptr_t cid) {
1660+
return is_external ?
1661+
0 :
1662+
(target::Instance::DataOffsetFor(cid) - kHeapObjectTag);
1663+
}
16571664

16581665
static int32_t EncodeImm26BranchOffset(int64_t imm, int32_t instr) {
16591666
const int32_t imm32 = static_cast<int32_t>(imm);

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

Lines changed: 43 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1365,9 +1365,8 @@ void LoadIndexedInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
13651365
compiler::Address element_address(TMP); // Bad address.
13661366
element_address =
13671367
index.IsRegister()
1368-
? __ ElementAddressForRegIndex(true, // Load.
1369-
IsExternal(), class_id(),
1370-
index_scale(), array, index.reg())
1368+
? __ ElementAddressForRegIndex(IsExternal(), class_id(),
1369+
index_scale(), array, index.reg(), TMP)
13711370
: __ ElementAddressForIntIndex(IsExternal(), class_id(),
13721371
index_scale(), array,
13731372
Smi::Cast(index.constant()).Value());
@@ -1499,7 +1498,7 @@ void LoadCodeUnitsInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
14991498
}
15001499
// Warning: element_address may use register TMP as base.
15011500
compiler::Address element_address = __ ElementAddressForRegIndexWithSize(
1502-
true, IsExternal(), class_id(), sz, index_scale(), str, index.reg());
1501+
IsExternal(), class_id(), sz, index_scale(), str, index.reg(), TMP);
15031502
__ ldr(result, element_address, sz);
15041503

15051504
__ SmiTag(result);
@@ -1555,7 +1554,7 @@ LocationSummary* StoreIndexedInstr::MakeLocationSummary(Zone* zone,
15551554
if (CanBeImmediateIndex(index(), class_id(), IsExternal())) {
15561555
locs->set_in(1, Location::Constant(index()->definition()->AsConstant()));
15571556
} else {
1558-
locs->set_in(1, Location::WritableRegister());
1557+
locs->set_in(1, Location::RequiresRegister());
15591558
}
15601559
locs->set_temp(0, Location::RequiresRegister());
15611560

@@ -1603,32 +1602,43 @@ void StoreIndexedInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
16031602
// The array register points to the backing store for external arrays.
16041603
const Register array = locs()->in(0).reg();
16051604
const Location index = locs()->in(1);
1606-
const Register address = locs()->temp(0).reg();
1605+
const Register temp = locs()->temp(0).reg();
1606+
compiler::Address element_address(TMP); // Bad address.
16071607

1608-
if (index.IsRegister()) {
1609-
__ LoadElementAddressForRegIndex(address,
1610-
false, // Store.
1611-
IsExternal(), class_id(), index_scale(),
1612-
array, index.reg());
1613-
} else {
1614-
__ LoadElementAddressForIntIndex(address, IsExternal(), class_id(),
1615-
index_scale(), array,
1616-
Smi::Cast(index.constant()).Value());
1608+
// Deal with a special case separately.
1609+
if (class_id() == kArrayCid && ShouldEmitStoreBarrier()) {
1610+
if (index.IsRegister()) {
1611+
__ ComputeElementAddressForRegIndex(temp, IsExternal(), class_id(),
1612+
index_scale(), array, index.reg());
1613+
} else {
1614+
__ ComputeElementAddressForIntIndex(temp, IsExternal(), class_id(),
1615+
index_scale(), array,
1616+
Smi::Cast(index.constant()).Value());
1617+
}
1618+
const Register value = locs()->in(2).reg();
1619+
__ StoreIntoArray(array, temp, value, CanValueBeSmi(),
1620+
/*lr_reserved=*/!compiler->intrinsic_mode());
1621+
return;
16171622
}
16181623

1624+
element_address =
1625+
index.IsRegister()
1626+
? __ ElementAddressForRegIndex(IsExternal(), class_id(),
1627+
index_scale(), array,
1628+
index.reg(), temp)
1629+
: __ ElementAddressForIntIndex(IsExternal(), class_id(),
1630+
index_scale(), array,
1631+
Smi::Cast(index.constant()).Value());
1632+
16191633
switch (class_id()) {
16201634
case kArrayCid:
1621-
if (ShouldEmitStoreBarrier()) {
1622-
const Register value = locs()->in(2).reg();
1623-
__ StoreIntoArray(array, address, value, CanValueBeSmi(),
1624-
/*lr_reserved=*/!compiler->intrinsic_mode());
1625-
} else if (locs()->in(2).IsConstant()) {
1635+
ASSERT(!ShouldEmitStoreBarrier()); // Specially treated above.
1636+
if (locs()->in(2).IsConstant()) {
16261637
const Object& constant = locs()->in(2).constant();
1627-
__ StoreIntoObjectNoBarrier(array, compiler::Address(address),
1628-
constant);
1638+
__ StoreIntoObjectNoBarrier(array, element_address, constant);
16291639
} else {
16301640
const Register value = locs()->in(2).reg();
1631-
__ StoreIntoObjectNoBarrier(array, compiler::Address(address), value);
1641+
__ StoreIntoObjectNoBarrier(array, element_address, value);
16321642
}
16331643
break;
16341644
case kTypedDataInt8ArrayCid:
@@ -1639,10 +1649,10 @@ void StoreIndexedInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
16391649
if (locs()->in(2).IsConstant()) {
16401650
const Smi& constant = Smi::Cast(locs()->in(2).constant());
16411651
__ LoadImmediate(TMP, static_cast<int8_t>(constant.Value()));
1642-
__ str(TMP, compiler::Address(address), kUnsignedByte);
1652+
__ str(TMP, element_address, kUnsignedByte);
16431653
} else {
16441654
const Register value = locs()->in(2).reg();
1645-
__ str(value, compiler::Address(address), kUnsignedByte);
1655+
__ str(value, element_address, kUnsignedByte);
16461656
}
16471657
break;
16481658
}
@@ -1659,51 +1669,51 @@ void StoreIndexedInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
16591669
value = 0;
16601670
}
16611671
__ LoadImmediate(TMP, static_cast<int8_t>(value));
1662-
__ str(TMP, compiler::Address(address), kUnsignedByte);
1672+
__ str(TMP, element_address, kUnsignedByte);
16631673
} else {
16641674
const Register value = locs()->in(2).reg();
16651675
// Clamp to 0x00 or 0xFF respectively.
16661676
__ CompareImmediate(value, 0xFF);
16671677
__ csetm(TMP, GT); // TMP = value > 0xFF ? -1 : 0.
16681678
__ csel(TMP, value, TMP, LS); // TMP = value in range ? value : TMP.
1669-
__ str(TMP, compiler::Address(address), kUnsignedByte);
1679+
__ str(TMP, element_address, kUnsignedByte);
16701680
}
16711681
break;
16721682
}
16731683
case kTypedDataInt16ArrayCid:
16741684
case kTypedDataUint16ArrayCid: {
16751685
ASSERT(RequiredInputRepresentation(2) == kUnboxedIntPtr);
16761686
const Register value = locs()->in(2).reg();
1677-
__ str(value, compiler::Address(address), kUnsignedHalfword);
1687+
__ str(value, element_address, kUnsignedHalfword);
16781688
break;
16791689
}
16801690
case kTypedDataInt32ArrayCid:
16811691
case kTypedDataUint32ArrayCid: {
16821692
const Register value = locs()->in(2).reg();
1683-
__ str(value, compiler::Address(address), kUnsignedWord);
1693+
__ str(value, element_address, kUnsignedWord);
16841694
break;
16851695
}
16861696
case kTypedDataInt64ArrayCid:
16871697
case kTypedDataUint64ArrayCid: {
16881698
const Register value = locs()->in(2).reg();
1689-
__ str(value, compiler::Address(address), kDoubleWord);
1699+
__ str(value, element_address, kDoubleWord);
16901700
break;
16911701
}
16921702
case kTypedDataFloat32ArrayCid: {
16931703
const VRegister value_reg = locs()->in(2).fpu_reg();
1694-
__ fstrs(value_reg, compiler::Address(address));
1704+
__ fstrs(value_reg, element_address);
16951705
break;
16961706
}
16971707
case kTypedDataFloat64ArrayCid: {
16981708
const VRegister value_reg = locs()->in(2).fpu_reg();
1699-
__ fstrd(value_reg, compiler::Address(address));
1709+
__ fstrd(value_reg, element_address);
17001710
break;
17011711
}
17021712
case kTypedDataFloat64x2ArrayCid:
17031713
case kTypedDataInt32x4ArrayCid:
17041714
case kTypedDataFloat32x4ArrayCid: {
17051715
const VRegister value_reg = locs()->in(2).fpu_reg();
1706-
__ fstrq(value_reg, compiler::Address(address));
1716+
__ fstrq(value_reg, element_address);
17071717
break;
17081718
}
17091719
default:

‎runtime/vm/compiler/stub_code_compiler_arm64.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -385,12 +385,12 @@ void StubCodeCompiler::GenerateJITCallbackTrampolines(
385385
__ LoadFieldFromOffset(R9, R9,
386386
compiler::target::GrowableObjectArray::data_offset());
387387
__ ldr(R9, __ ElementAddressForRegIndex(
388-
/*is_load=*/true,
389388
/*external=*/false,
390389
/*array_cid=*/kArrayCid,
391390
/*index, smi-tagged=*/compiler::target::kWordSize * 2,
392391
/*array=*/R9,
393-
/*index=*/R8));
392+
/*index=*/R8,
393+
/*temp=*/TMP));
394394
__ LoadFieldFromOffset(R9, R9, compiler::target::Code::entry_point_offset());
395395

396396
// Clobbers all volatile registers, including the callback ID in R8.
@@ -1430,7 +1430,7 @@ void StubCodeCompiler::GenerateInvokeDartCodeStub(Assembler* assembler) {
14301430

14311431

14321432
#if defined(TARGET_OS_FUCHSIA)
1433-
__ mov (R3, THR);
1433+
__ mov(R3, THR);
14341434
#endif
14351435

14361436
__ PopNativeCalleeSavedRegisters(); // Clobbers THR
@@ -1569,7 +1569,7 @@ void StubCodeCompiler::GenerateInvokeDartCodeFromBytecodeStub(
15691569
__ StoreToOffset(R4, THR, target::Thread::vm_tag_offset());
15701570

15711571
#if defined(TARGET_OS_FUCHSIA)
1572-
__ mov (R3, THR);
1572+
__ mov(R3, THR);
15731573
#endif
15741574

15751575
__ PopNativeCalleeSavedRegisters(); // Clobbers THR

0 commit comments

Comments
 (0)