Skip to content

Commit 4081235

Browse files
dcharkescommit-bot@chromium.org
dcharkes
authored and
[email protected]
committed
[vm] Support unboxed indices in indexed load and store
Use unboxed indices in FFI loads, eliminating extra boxing and a shift for 1-byte loads. Speeds up Pointer<Int8,Int64,etc> loads/stores by 20-25% in AOT on x64. This does not use the unboxed indices in TypedData yet, using unboxed in TypedData without making the bounds checks unboxed makes it slower instead of faster. Issue: #39432 Change-Id: I2521fcd319cf4ed6891622e8a3c8c1924e9b01f3 Cq-Include-Trybots: luci.dart.try:vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64-try,app-kernel-linux-debug-x64-try,vm-kernel-linux-debug-ia32-try,vm-kernel-win-debug-x64-try,vm-kernel-win-debug-ia32-try,vm-kernel-precomp-linux-debug-x64-try,vm-dartkb-linux-release-x64-abi-try,vm-kernel-precomp-android-release-arm64-try,vm-kernel-asan-linux-release-x64-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-simarm64-try,vm-kernel-precomp-android-release-arm_x64-try,vm-kernel-precomp-obfuscate-linux-release-x64-try,dart-sdk-linux-try,analyzer-analysis-server-linux-try,analyzer-linux-release-try,front-end-linux-release-x64-try,vm-kernel-precomp-win-release-x64-try,vm-kernel-mac-debug-x64-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/137788 Reviewed-by: Martin Kustermann <[email protected]>
1 parent 9983424 commit 4081235

26 files changed

+325
-200
lines changed

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3679,10 +3679,12 @@ Address Assembler::ElementAddressForRegIndex(bool is_load,
36793679
bool is_external,
36803680
intptr_t cid,
36813681
intptr_t index_scale,
3682+
bool index_unboxed,
36823683
Register array,
36833684
Register index) {
3684-
// Note that index is expected smi-tagged, (i.e, LSL 1) for all arrays.
3685-
const intptr_t shift = Utils::ShiftForPowerOfTwo(index_scale) - kSmiTagShift;
3685+
// If unboxed, index is expected smi-tagged, (i.e, LSL 1) for all arrays.
3686+
const intptr_t boxing_shift = index_unboxed ? 0 : -kSmiTagShift;
3687+
const intptr_t shift = Utils::ShiftForPowerOfTwo(index_scale) + boxing_shift;
36863688
int32_t offset =
36873689
is_external ? 0 : (target::Instance::DataOffsetFor(cid) - kHeapObjectTag);
36883690
const OperandSize size = Address::OperandSizeFor(cid);
@@ -3720,10 +3722,12 @@ void Assembler::LoadElementAddressForRegIndex(Register address,
37203722
bool is_external,
37213723
intptr_t cid,
37223724
intptr_t index_scale,
3725+
bool index_unboxed,
37233726
Register array,
37243727
Register index) {
3725-
// Note that index is expected smi-tagged, (i.e, LSL 1) for all arrays.
3726-
const intptr_t shift = Utils::ShiftForPowerOfTwo(index_scale) - kSmiTagShift;
3728+
// If unboxed, index is expected smi-tagged, (i.e, LSL 1) for all arrays.
3729+
const intptr_t boxing_shift = index_unboxed ? 0 : -kSmiTagShift;
3730+
const intptr_t shift = Utils::ShiftForPowerOfTwo(index_scale) + boxing_shift;
37273731
int32_t offset =
37283732
is_external ? 0 : (target::Instance::DataOffsetFor(cid) - kHeapObjectTag);
37293733
if (shift < 0) {

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1145,6 +1145,7 @@ class Assembler : public AssemblerBase {
11451145
bool is_external,
11461146
intptr_t cid,
11471147
intptr_t index_scale,
1148+
bool index_unboxed,
11481149
Register array,
11491150
Register index);
11501151

@@ -1153,6 +1154,7 @@ class Assembler : public AssemblerBase {
11531154
bool is_external,
11541155
intptr_t cid,
11551156
intptr_t index_scale,
1157+
bool index_unboxed,
11561158
Register array,
11571159
Register index);
11581160

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

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1762,23 +1762,26 @@ void Assembler::ComputeElementAddressForIntIndex(Register address,
17621762
Address Assembler::ElementAddressForRegIndex(bool is_external,
17631763
intptr_t cid,
17641764
intptr_t index_scale,
1765+
bool index_unboxed,
17651766
Register array,
17661767
Register index,
17671768
Register temp) {
1768-
return ElementAddressForRegIndexWithSize(is_external, cid,
1769-
Address::OperandSizeFor(cid),
1770-
index_scale, array, index, temp);
1769+
return ElementAddressForRegIndexWithSize(
1770+
is_external, cid, Address::OperandSizeFor(cid), index_scale,
1771+
index_unboxed, array, index, temp);
17711772
}
17721773

17731774
Address Assembler::ElementAddressForRegIndexWithSize(bool is_external,
17741775
intptr_t cid,
17751776
OperandSize size,
17761777
intptr_t index_scale,
1778+
bool index_unboxed,
17771779
Register array,
17781780
Register index,
17791781
Register temp) {
1780-
// Note that index is expected smi-tagged, (i.e, LSL 1) for all arrays.
1781-
const intptr_t shift = Utils::ShiftForPowerOfTwo(index_scale) - kSmiTagShift;
1782+
// If unboxed, index is expected smi-tagged, (i.e, LSL 1) for all arrays.
1783+
const intptr_t boxing_shift = index_unboxed ? 0 : -kSmiTagShift;
1784+
const intptr_t shift = Utils::ShiftForPowerOfTwo(index_scale) + boxing_shift;
17821785
const int32_t offset = HeapDataOffset(is_external, cid);
17831786
ASSERT(array != temp);
17841787
ASSERT(index != temp);
@@ -1798,10 +1801,12 @@ void Assembler::ComputeElementAddressForRegIndex(Register address,
17981801
bool is_external,
17991802
intptr_t cid,
18001803
intptr_t index_scale,
1804+
bool index_unboxed,
18011805
Register array,
18021806
Register index) {
1803-
// Note that index is expected smi-tagged, (i.e, LSL 1) for all arrays.
1804-
const intptr_t shift = Utils::ShiftForPowerOfTwo(index_scale) - kSmiTagShift;
1807+
// If unboxed, index is expected smi-tagged, (i.e, LSL 1) for all arrays.
1808+
const intptr_t boxing_shift = index_unboxed ? 0 : -kSmiTagShift;
1809+
const intptr_t shift = Utils::ShiftForPowerOfTwo(index_scale) + boxing_shift;
18051810
const int32_t offset = HeapDataOffset(is_external, cid);
18061811
if (shift == 0) {
18071812
add(address, array, Operand(index));

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1676,6 +1676,7 @@ class Assembler : public AssemblerBase {
16761676
Address ElementAddressForRegIndex(bool is_external,
16771677
intptr_t cid,
16781678
intptr_t index_scale,
1679+
bool index_unboxed,
16791680
Register array,
16801681
Register index,
16811682
Register temp);
@@ -1687,6 +1688,7 @@ class Assembler : public AssemblerBase {
16871688
intptr_t cid,
16881689
OperandSize size,
16891690
intptr_t index_scale,
1691+
bool index_unboxed,
16901692
Register array,
16911693
Register index,
16921694
Register temp);
@@ -1695,6 +1697,7 @@ class Assembler : public AssemblerBase {
16951697
bool is_external,
16961698
intptr_t cid,
16971699
intptr_t index_scale,
1700+
bool index_unboxed,
16981701
Register array,
16991702
Register index);
17001703

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

Lines changed: 41 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2735,38 +2735,58 @@ Address Assembler::ElementAddressForIntIndex(bool is_external,
27352735
}
27362736
}
27372737

2738-
static ScaleFactor ToScaleFactor(intptr_t index_scale) {
2739-
// Note that index is expected smi-tagged, (i.e, times 2) for all arrays with
2740-
// index scale factor > 1. E.g., for Uint8Array and OneByteString the index is
2741-
// expected to be untagged before accessing.
2742-
ASSERT(kSmiTagShift == 1);
2743-
switch (index_scale) {
2744-
case 1:
2745-
return TIMES_1;
2746-
case 2:
2747-
return TIMES_1;
2748-
case 4:
2749-
return TIMES_2;
2750-
case 8:
2751-
return TIMES_4;
2752-
case 16:
2753-
return TIMES_8;
2754-
default:
2755-
UNREACHABLE();
2756-
return TIMES_1;
2738+
static ScaleFactor ToScaleFactor(intptr_t index_scale, bool index_unboxed) {
2739+
if (index_unboxed) {
2740+
switch (index_scale) {
2741+
case 1:
2742+
return TIMES_1;
2743+
case 2:
2744+
return TIMES_2;
2745+
case 4:
2746+
return TIMES_4;
2747+
case 8:
2748+
return TIMES_8;
2749+
case 16:
2750+
return TIMES_16;
2751+
default:
2752+
UNREACHABLE();
2753+
return TIMES_1;
2754+
}
2755+
} else {
2756+
// Note that index is expected smi-tagged, (i.e, times 2) for all arrays
2757+
// with index scale factor > 1. E.g., for Uint8Array and OneByteString the
2758+
// index is expected to be untagged before accessing.
2759+
ASSERT(kSmiTagShift == 1);
2760+
switch (index_scale) {
2761+
case 1:
2762+
return TIMES_1;
2763+
case 2:
2764+
return TIMES_1;
2765+
case 4:
2766+
return TIMES_2;
2767+
case 8:
2768+
return TIMES_4;
2769+
case 16:
2770+
return TIMES_8;
2771+
default:
2772+
UNREACHABLE();
2773+
return TIMES_1;
2774+
}
27572775
}
27582776
}
27592777

27602778
Address Assembler::ElementAddressForRegIndex(bool is_external,
27612779
intptr_t cid,
27622780
intptr_t index_scale,
2781+
bool index_unboxed,
27632782
Register array,
27642783
Register index,
27652784
intptr_t extra_disp) {
27662785
if (is_external) {
2767-
return Address(array, index, ToScaleFactor(index_scale), extra_disp);
2786+
return Address(array, index, ToScaleFactor(index_scale, index_unboxed),
2787+
extra_disp);
27682788
} else {
2769-
return FieldAddress(array, index, ToScaleFactor(index_scale),
2789+
return FieldAddress(array, index, ToScaleFactor(index_scale, index_unboxed),
27702790
target::Instance::DataOffsetFor(cid) + extra_disp);
27712791
}
27722792
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ class Address : public Operand {
153153

154154
Address(Register index, ScaleFactor scale, int32_t disp) {
155155
ASSERT(index != ESP); // Illegal addressing mode.
156+
ASSERT(scale != TIMES_16); // Unsupported scale factor.
156157
SetModRM(0, ESP);
157158
SetSIB(scale, index, EBP);
158159
SetDisp32(disp);
@@ -163,6 +164,7 @@ class Address : public Operand {
163164

164165
Address(Register base, Register index, ScaleFactor scale, int32_t disp) {
165166
ASSERT(index != ESP); // Illegal addressing mode.
167+
ASSERT(scale != TIMES_16); // Unsupported scale factor.
166168
if (disp == 0 && base != EBP) {
167169
SetModRM(0, ESP);
168170
SetSIB(scale, index, base);
@@ -732,6 +734,7 @@ class Assembler : public AssemblerBase {
732734
static Address ElementAddressForRegIndex(bool is_external,
733735
intptr_t cid,
734736
intptr_t index_scale,
737+
bool index_unboxed,
735738
Register array,
736739
Register index,
737740
intptr_t extra_disp = 0);

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

Lines changed: 40 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2229,37 +2229,56 @@ Address Assembler::ElementAddressForIntIndex(bool is_external,
22292229
}
22302230
}
22312231

2232-
static ScaleFactor ToScaleFactor(intptr_t index_scale) {
2233-
// Note that index is expected smi-tagged, (i.e, times 2) for all arrays with
2234-
// index scale factor > 1. E.g., for Uint8Array and OneByteString the index is
2235-
// expected to be untagged before accessing.
2236-
ASSERT(kSmiTagShift == 1);
2237-
switch (index_scale) {
2238-
case 1:
2239-
return TIMES_1;
2240-
case 2:
2241-
return TIMES_1;
2242-
case 4:
2243-
return TIMES_2;
2244-
case 8:
2245-
return TIMES_4;
2246-
case 16:
2247-
return TIMES_8;
2248-
default:
2249-
UNREACHABLE();
2250-
return TIMES_1;
2232+
static ScaleFactor ToScaleFactor(intptr_t index_scale, bool index_unboxed) {
2233+
if (index_unboxed) {
2234+
switch (index_scale) {
2235+
case 1:
2236+
return TIMES_1;
2237+
case 2:
2238+
return TIMES_2;
2239+
case 4:
2240+
return TIMES_4;
2241+
case 8:
2242+
return TIMES_8;
2243+
case 16:
2244+
return TIMES_16;
2245+
default:
2246+
UNREACHABLE();
2247+
return TIMES_1;
2248+
}
2249+
} else {
2250+
// Note that index is expected smi-tagged, (i.e, times 2) for all arrays
2251+
// with index scale factor > 1. E.g., for Uint8Array and OneByteString the
2252+
// index is expected to be untagged before accessing.
2253+
ASSERT(kSmiTagShift == 1);
2254+
switch (index_scale) {
2255+
case 1:
2256+
return TIMES_1;
2257+
case 2:
2258+
return TIMES_1;
2259+
case 4:
2260+
return TIMES_2;
2261+
case 8:
2262+
return TIMES_4;
2263+
case 16:
2264+
return TIMES_8;
2265+
default:
2266+
UNREACHABLE();
2267+
return TIMES_1;
2268+
}
22512269
}
22522270
}
22532271

22542272
Address Assembler::ElementAddressForRegIndex(bool is_external,
22552273
intptr_t cid,
22562274
intptr_t index_scale,
2275+
bool index_unboxed,
22572276
Register array,
22582277
Register index) {
22592278
if (is_external) {
2260-
return Address(array, index, ToScaleFactor(index_scale), 0);
2279+
return Address(array, index, ToScaleFactor(index_scale, index_unboxed), 0);
22612280
} else {
2262-
return FieldAddress(array, index, ToScaleFactor(index_scale),
2281+
return FieldAddress(array, index, ToScaleFactor(index_scale, index_unboxed),
22632282
target::Instance::DataOffsetFor(cid));
22642283
}
22652284
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@ class Address : public Operand {
192192

193193
Address(Register index, ScaleFactor scale, int32_t disp) {
194194
ASSERT(index != RSP); // Illegal addressing mode.
195+
ASSERT(scale != TIMES_16); // Unsupported scale factor.
195196
SetModRM(0, RSP);
196197
SetSIB(scale, index, RBP);
197198
SetDisp32(disp);
@@ -202,6 +203,7 @@ class Address : public Operand {
202203

203204
Address(Register base, Register index, ScaleFactor scale, int32_t disp) {
204205
ASSERT(index != RSP); // Illegal addressing mode.
206+
ASSERT(scale != TIMES_16); // Unsupported scale factor.
205207
if ((disp == 0) && ((base & 7) != RBP)) {
206208
SetModRM(0, RSP);
207209
SetSIB(scale, index, base);
@@ -962,6 +964,7 @@ class Assembler : public AssemblerBase {
962964
static Address ElementAddressForRegIndex(bool is_external,
963965
intptr_t cid,
964966
intptr_t index_scale,
967+
bool index_unboxed,
965968
Register array,
966969
Register index);
967970

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5278,13 +5278,15 @@ static AlignmentType StrengthenAlignment(intptr_t cid,
52785278

52795279
LoadIndexedInstr::LoadIndexedInstr(Value* array,
52805280
Value* index,
5281+
bool index_unboxed,
52815282
intptr_t index_scale,
52825283
intptr_t class_id,
52835284
AlignmentType alignment,
52845285
intptr_t deopt_id,
52855286
TokenPosition token_pos,
52865287
CompileType* result_type)
52875288
: TemplateDefinition(deopt_id),
5289+
index_unboxed_(index_unboxed),
52885290
index_scale_(index_scale),
52895291
class_id_(class_id),
52905292
alignment_(StrengthenAlignment(class_id, alignment)),
@@ -5298,6 +5300,7 @@ StoreIndexedInstr::StoreIndexedInstr(Value* array,
52985300
Value* index,
52995301
Value* value,
53005302
StoreBarrierType emit_store_barrier,
5303+
bool index_unboxed,
53015304
intptr_t index_scale,
53025305
intptr_t class_id,
53035306
AlignmentType alignment,
@@ -5306,6 +5309,7 @@ StoreIndexedInstr::StoreIndexedInstr(Value* array,
53065309
SpeculativeMode speculative_mode)
53075310
: TemplateInstruction(deopt_id),
53085311
emit_store_barrier_(emit_store_barrier),
5312+
index_unboxed_(index_unboxed),
53095313
index_scale_(index_scale),
53105314
class_id_(class_id),
53115315
alignment_(StrengthenAlignment(class_id, alignment)),

0 commit comments

Comments
 (0)