Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 23ae4fa

Browse files
aamaam
committed
Revert "[VM] Reduce Smi size to 32 bit on 64 bit platforms"
This reverts commit 0e9a77a as it introduces regression in Flutter build: https://build.chromium.org/p/client.flutter/builders/Mac%20Engine/builds/2573/steps/build%20ios_debug_arm/logs/stdio Revert d58b0e2 Change-Id: I1c4a9384d4fa8cc809412b1d22409221771acda0 Reviewed-on: https://dart-review.googlesource.com/50600 Reviewed-by: Alexander Markov <[email protected]> Commit-Queue: Alexander Aprelev <[email protected]>
1 parent 6ab6d1e commit 23ae4fa

25 files changed

+567
-949
lines changed

‎runtime/lib/compact_hash.dart

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,14 +78,19 @@ abstract class _HashBase implements _HashVMBase {
7878
static const int _UNUSED_PAIR = 0;
7979
static const int _DELETED_PAIR = 1;
8080

81-
// The top bits are wasted to avoid Mint allocation.
81+
// On 32-bit, the top bits are wasted to avoid Mint allocation.
82+
// TODO(koda): Reclaim the bits by making the compiler treat hash patterns
83+
// as unsigned words.
8284
static int _indexSizeToHashMask(int indexSize) {
8385
int indexBits = indexSize.bitLength - 2;
84-
return (1 << (30 - indexBits)) - 1;
86+
return internal.is64Bit
87+
? (1 << (32 - indexBits)) - 1
88+
: (1 << (30 - indexBits)) - 1;
8589
}
8690

8791
static int _hashPattern(int fullHash, int hashMask, int size) {
8892
final int maskedHash = fullHash & hashMask;
93+
// TODO(koda): Consider keeping bit length and use left shift.
8994
return (maskedHash == 0) ? (size >> 1) : maskedHash * (size >> 1);
9095
}
9196

‎runtime/lib/integers_patch.dart

Lines changed: 51 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ class int {
3232
return null; // Empty.
3333
}
3434
}
35-
var smiLimit = 9;
35+
var smiLimit = is64Bit ? 18 : 9;
3636
if ((last - ix) >= smiLimit) {
3737
return null; // May not fit into a Smi.
3838
}
@@ -117,7 +117,7 @@ class int {
117117

118118
static int _parseRadix(
119119
String source, int radix, int start, int end, int sign) {
120-
int tableIndex = (radix - 2) * 2;
120+
int tableIndex = (radix - 2) * 4 + (is64Bit ? 2 : 0);
121121
int blockSize = _PARSE_LIMITS[tableIndex];
122122
int length = end - start;
123123
if (length <= blockSize) {
@@ -143,7 +143,7 @@ class int {
143143
int positiveOverflowLimit = 0;
144144
int negativeOverflowLimit = 0;
145145
if (_limitIntsTo64Bits) {
146-
tableIndex = tableIndex << 1; // Pre-multiply by 2 for simpler indexing.
146+
tableIndex = tableIndex << 1; // pre-multiply by 2 for simpler indexing
147147
positiveOverflowLimit = _int64OverflowLimits[tableIndex];
148148
if (positiveOverflowLimit == 0) {
149149
positiveOverflowLimit =
@@ -159,10 +159,14 @@ class int {
159159
if (result >= positiveOverflowLimit) {
160160
if ((result > positiveOverflowLimit) ||
161161
(smi > _int64OverflowLimits[tableIndex + 2])) {
162+
// Although the unsigned overflow limits do not depend on the
163+
// platform, the multiplier and block size, which are used to
164+
// compute it, do.
165+
int X = is64Bit ? 1 : 0;
162166
if (radix == 16 &&
163-
!(result >= _int64UnsignedOverflowLimit &&
164-
(result > _int64UnsignedOverflowLimit ||
165-
smi > _int64UnsignedSmiOverflowLimit)) &&
167+
!(result >= _int64UnsignedOverflowLimits[X] &&
168+
(result > _int64UnsignedOverflowLimits[X] ||
169+
smi > _int64UnsignedSmiOverflowLimits[X])) &&
166170
blockEnd + blockSize > end) {
167171
return (result * multiplier) + smi;
168172
}
@@ -207,42 +211,43 @@ class int {
207211

208212
// For each radix, 2-36, how many digits are guaranteed to fit in a smi,
209213
// and magnitude of such a block (radix ** digit-count).
214+
// 32-bit limit/multiplier at (radix - 2)*4, 64-bit limit at (radix-2)*4+2
210215
static const _PARSE_LIMITS = const [
211-
30, 1073741824, // radix: 2
212-
18, 387420489,
213-
15, 1073741824,
214-
12, 244140625, // radix: 5
215-
11, 362797056,
216-
10, 282475249,
217-
10, 1073741824,
218-
9, 387420489,
219-
9, 1000000000, // radix: 10
220-
8, 214358881,
221-
8, 429981696,
222-
8, 815730721,
223-
7, 105413504,
224-
7, 170859375, // radix: 15
225-
7, 268435456,
226-
7, 410338673,
227-
7, 612220032,
228-
7, 893871739,
229-
6, 64000000, // radix: 20
230-
6, 85766121,
231-
6, 113379904,
232-
6, 148035889,
233-
6, 191102976,
234-
6, 244140625, // radix: 25
235-
6, 308915776,
236-
6, 387420489,
237-
6, 481890304,
238-
6, 594823321,
239-
6, 729000000, // radix: 30
240-
6, 887503681,
241-
6, 1073741824,
242-
5, 39135393,
243-
5, 45435424,
244-
5, 52521875, // radix: 35
245-
5, 60466176,
216+
30, 1073741824, 62, 4611686018427387904, // radix: 2
217+
18, 387420489, 39, 4052555153018976267,
218+
15, 1073741824, 30, 1152921504606846976,
219+
12, 244140625, 26, 1490116119384765625, // radix: 5
220+
11, 362797056, 23, 789730223053602816,
221+
10, 282475249, 22, 3909821048582988049,
222+
10, 1073741824, 20, 1152921504606846976,
223+
9, 387420489, 19, 1350851717672992089,
224+
9, 1000000000, 18, 1000000000000000000, // radix: 10
225+
8, 214358881, 17, 505447028499293771,
226+
8, 429981696, 17, 2218611106740436992,
227+
8, 815730721, 16, 665416609183179841,
228+
7, 105413504, 16, 2177953337809371136,
229+
7, 170859375, 15, 437893890380859375, // radix: 15
230+
7, 268435456, 15, 1152921504606846976,
231+
7, 410338673, 15, 2862423051509815793,
232+
7, 612220032, 14, 374813367582081024,
233+
7, 893871739, 14, 799006685782884121,
234+
6, 64000000, 14, 1638400000000000000, // radix: 20
235+
6, 85766121, 14, 3243919932521508681,
236+
6, 113379904, 13, 282810057883082752,
237+
6, 148035889, 13, 504036361936467383,
238+
6, 191102976, 13, 876488338465357824,
239+
6, 244140625, 13, 1490116119384765625, // radix: 25
240+
6, 308915776, 13, 2481152873203736576,
241+
6, 387420489, 13, 4052555153018976267,
242+
6, 481890304, 12, 232218265089212416,
243+
6, 594823321, 12, 353814783205469041,
244+
6, 729000000, 12, 531441000000000000, // radix: 30
245+
6, 887503681, 12, 787662783788549761,
246+
6, 1073741824, 12, 1152921504606846976,
247+
5, 39135393, 12, 1667889514952984961,
248+
5, 45435424, 12, 2386420683693101056,
249+
5, 52521875, 12, 3379220508056640625, // radix: 35
250+
5, 60466176, 11, 131621703842267136,
246251
];
247252

248253
/// Flag indicating if integers are limited by 64 bits
@@ -252,8 +257,11 @@ class int {
252257
static const _maxInt64 = 0x7fffffffffffffff;
253258
static const _minInt64 = -_maxInt64 - 1;
254259

255-
static const _int64UnsignedOverflowLimit = 0xfffffffff;
256-
static const _int64UnsignedSmiOverflowLimit = 0xfffffff;
260+
static const _int64UnsignedOverflowLimits = const [0xfffffffff, 0xf];
261+
static const _int64UnsignedSmiOverflowLimits = const [
262+
0xfffffff,
263+
0xfffffffffffffff
264+
];
257265

258266
/// In the `--limit-ints-to-64-bits` mode calculation of the expression
259267
///

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

Lines changed: 3 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,18 +1069,12 @@ class Assembler : public ValueObject {
10691069
const Register crn = ConcreteRegister(rn);
10701070
EmitFPIntCvtOp(SCVTFD, static_cast<Register>(vd), crn, kWord);
10711071
}
1072-
void fcvtzdsx(Register rd, VRegister vn) {
1072+
void fcvtzds(Register rd, VRegister vn) {
10731073
ASSERT(rd != R31);
10741074
ASSERT(rd != CSP);
10751075
const Register crd = ConcreteRegister(rd);
10761076
EmitFPIntCvtOp(FCVTZDS, crd, static_cast<Register>(vn));
10771077
}
1078-
void fcvtzdsw(Register rd, VRegister vn) {
1079-
ASSERT(rd != R31);
1080-
ASSERT(rd != CSP);
1081-
const Register crd = ConcreteRegister(rd);
1082-
EmitFPIntCvtOp(FCVTZDS, crd, static_cast<Register>(vn), kWord);
1083-
}
10841078
void fmovdd(VRegister vd, VRegister vn) { EmitFPOneSourceOp(FMOVDD, vd, vn); }
10851079
void fabsd(VRegister vd, VRegister vn) { EmitFPOneSourceOp(FABSD, vd, vn); }
10861080
void fnegd(VRegister vd, VRegister vn) { EmitFPOneSourceOp(FNEGD, vd, vn); }
@@ -1371,17 +1365,9 @@ class Assembler : public ValueObject {
13711365
LslImmediate(dst, src, kSmiTagSize);
13721366
}
13731367

1374-
void BranchIfNotSmi(Register reg, Label* label) {
1375-
ASSERT(kSmiTagMask == 1);
1376-
ASSERT(kSmiTag == 0);
1377-
tbnz(label, reg, 0);
1378-
}
1368+
void BranchIfNotSmi(Register reg, Label* label) { tbnz(label, reg, kSmiTag); }
13791369

1380-
void BranchIfSmi(Register reg, Label* label) {
1381-
ASSERT(kSmiTagMask == 1);
1382-
ASSERT(kSmiTag == 0);
1383-
tbz(label, reg, 0);
1384-
}
1370+
void BranchIfSmi(Register reg, Label* label) { tbz(label, reg, kSmiTag); }
13851371

13861372
void Branch(const StubEntry& stub_entry,
13871373
Register pp,
@@ -1465,11 +1451,6 @@ class Assembler : public ValueObject {
14651451
kValueCanBeSmi,
14661452
};
14671453

1468-
enum CanBeHeapPointer {
1469-
kValueIsNotHeapPointer,
1470-
kValueCanBeHeapPointer,
1471-
};
1472-
14731454
// Storing into an object.
14741455
void StoreIntoObject(Register object,
14751456
const Address& dest,
@@ -1610,22 +1591,6 @@ class Assembler : public ValueObject {
16101591
Register tmp,
16111592
OperandSize sz);
16121593

1613-
void AssertSmiInRange(
1614-
Register object,
1615-
CanBeHeapPointer can_be_heap_pointer = kValueIsNotHeapPointer) {
1616-
#if defined(DEBUG)
1617-
Label ok;
1618-
if (can_be_heap_pointer == kValueCanBeHeapPointer) {
1619-
BranchIfNotSmi(object, &ok);
1620-
}
1621-
cmp(object, Operand(object, SXTW, 0));
1622-
b(&ok, EQ);
1623-
Stop("Smi out of range");
1624-
1625-
Bind(&ok);
1626-
#endif
1627-
}
1628-
16291594
private:
16301595
AssemblerBuffer buffer_; // Contains position independent code.
16311596
ObjectPoolWrapper object_pool_wrapper_;

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

Lines changed: 3 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -2576,73 +2576,17 @@ ASSEMBLER_TEST_RUN(FldrqFstrqPrePostIndex, test) {
25762576
EXPECT_EQ(42.0, EXECUTE_TEST_CODE_DOUBLE(DoubleReturn, test->entry()));
25772577
}
25782578

2579-
ASSEMBLER_TEST_GENERATE(Fcvtzdsx, assembler) {
2579+
ASSEMBLER_TEST_GENERATE(Fcvtzds, assembler) {
25802580
__ LoadDImmediate(V0, 42.0);
2581-
__ fcvtzdsx(R0, V0);
2581+
__ fcvtzds(R0, V0);
25822582
__ ret();
25832583
}
25842584

2585-
ASSEMBLER_TEST_RUN(Fcvtzdsx, test) {
2585+
ASSEMBLER_TEST_RUN(Fcvtzds, test) {
25862586
typedef int64_t (*Int64Return)() DART_UNUSED;
25872587
EXPECT_EQ(42, EXECUTE_TEST_CODE_INT64(Int64Return, test->entry()));
25882588
}
25892589

2590-
ASSEMBLER_TEST_GENERATE(Fcvtzdsw, assembler) {
2591-
__ LoadDImmediate(V0, 42.0);
2592-
__ fcvtzdsw(R0, V0);
2593-
__ ret();
2594-
}
2595-
2596-
ASSEMBLER_TEST_RUN(Fcvtzdsw, test) {
2597-
typedef int64_t (*Int64Return)() DART_UNUSED;
2598-
EXPECT_EQ(42, EXECUTE_TEST_CODE_INT64(Int64Return, test->entry()));
2599-
}
2600-
2601-
ASSEMBLER_TEST_GENERATE(Fcvtzdsx_overflow, assembler) {
2602-
__ LoadDImmediate(V0, 1e20);
2603-
__ fcvtzdsx(R0, V0);
2604-
__ ret();
2605-
}
2606-
2607-
ASSEMBLER_TEST_RUN(Fcvtzdsx_overflow, test) {
2608-
typedef int64_t (*Int64Return)() DART_UNUSED;
2609-
EXPECT_EQ(kMaxInt64, EXECUTE_TEST_CODE_INT64(Int64Return, test->entry()));
2610-
}
2611-
2612-
ASSEMBLER_TEST_GENERATE(Fcvtzdsx_overflow_negative, assembler) {
2613-
__ LoadDImmediate(V0, -1e20);
2614-
__ fcvtzdsx(R0, V0);
2615-
__ ret();
2616-
}
2617-
2618-
ASSEMBLER_TEST_RUN(Fcvtzdsx_overflow_negative, test) {
2619-
typedef int64_t (*Int64Return)() DART_UNUSED;
2620-
EXPECT_EQ(kMinInt64, EXECUTE_TEST_CODE_INT64(Int64Return, test->entry()));
2621-
}
2622-
2623-
ASSEMBLER_TEST_GENERATE(Fcvtzdsw_overflow, assembler) {
2624-
__ LoadDImmediate(V0, 1e10);
2625-
__ fcvtzdsw(R0, V0);
2626-
__ ret();
2627-
}
2628-
2629-
ASSEMBLER_TEST_RUN(Fcvtzdsw_overflow, test) {
2630-
typedef int64_t (*Int64Return)() DART_UNUSED;
2631-
EXPECT_EQ(kMaxInt32, EXECUTE_TEST_CODE_INT64(Int64Return, test->entry()));
2632-
}
2633-
2634-
ASSEMBLER_TEST_GENERATE(Fcvtzdsw_overflow_negative, assembler) {
2635-
__ LoadDImmediate(V0, -1e10);
2636-
__ fcvtzdsw(R0, V0);
2637-
__ sxtw(R0, R0);
2638-
__ ret();
2639-
}
2640-
2641-
ASSEMBLER_TEST_RUN(Fcvtzdsw_overflow_negative, test) {
2642-
typedef int64_t (*Int64Return)() DART_UNUSED;
2643-
EXPECT_EQ(kMinInt32, EXECUTE_TEST_CODE_INT64(Int64Return, test->entry()));
2644-
}
2645-
26462590
ASSEMBLER_TEST_GENERATE(Scvtfdx, assembler) {
26472591
__ LoadImmediate(R0, 42);
26482592
__ scvtfdx(V0, R0);

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

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -521,10 +521,7 @@ class Assembler : public ValueObject {
521521
return CompareImmediate(reg, Immediate(immediate));
522522
}
523523

524-
void testl(Register reg, const Immediate& imm) {
525-
Immediate imm2(imm.value() & 0xffffffffll);
526-
testq(reg, imm2);
527-
}
524+
void testl(Register reg, const Immediate& imm) { testq(reg, imm); }
528525
void testb(const Address& address, const Immediate& imm);
529526

530527
void testq(Register reg, const Immediate& imm);
@@ -713,11 +710,6 @@ class Assembler : public ValueObject {
713710
kValueCanBeSmi,
714711
};
715712

716-
enum CanBeHeapPointer {
717-
kValueIsNotHeapPointer,
718-
kValueCanBeHeapPointer,
719-
};
720-
721713
// Destroys value.
722714
void StoreIntoObject(Register object, // Object we are storing into.
723715
const Address& dest, // Where we are storing into.
@@ -947,26 +939,6 @@ class Assembler : public ValueObject {
947939
Register array,
948940
Register index);
949941

950-
void AssertSmiInRange(
951-
Register object,
952-
CanBeHeapPointer can_be_heap_pointer = kValueIsNotHeapPointer) {
953-
#if defined(DEBUG)
954-
Register tmp = object == TMP ? TMP2 : TMP;
955-
Label ok;
956-
if (can_be_heap_pointer == kValueCanBeHeapPointer) {
957-
testl(object, Immediate(kSmiTagMask));
958-
ASSERT(kSmiTag == 0);
959-
j(ZERO, &ok);
960-
}
961-
movsxd(tmp, object);
962-
cmpq(tmp, object);
963-
j(EQUAL, &ok);
964-
Stop("Smi out of range");
965-
966-
Bind(&ok);
967-
#endif
968-
}
969-
970942
static Address VMTagAddress() {
971943
return Address(THR, Thread::vm_tag_offset());
972944
}

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1584,10 +1584,10 @@ bool BinaryIntegerOpInstr::RightIsPowerOfTwoConstant() const {
15841584
return Utils::IsPowerOfTwo(Utils::Abs(int_value));
15851585
}
15861586

1587-
static intptr_t SignificantRepresentationBits(Representation r) {
1587+
static intptr_t RepresentationBits(Representation r) {
15881588
switch (r) {
15891589
case kTagged:
1590-
return 31;
1590+
return kBitsPerWord - 1;
15911591
case kUnboxedInt32:
15921592
case kUnboxedUint32:
15931593
return 32;
@@ -1601,7 +1601,7 @@ static intptr_t SignificantRepresentationBits(Representation r) {
16011601

16021602
static int64_t RepresentationMask(Representation r) {
16031603
return static_cast<int64_t>(static_cast<uint64_t>(-1) >>
1604-
(64 - SignificantRepresentationBits(r)));
1604+
(64 - RepresentationBits(r)));
16051605
}
16061606

16071607
static bool ToIntegerConstant(Value* value, int64_t* result) {
@@ -2162,8 +2162,7 @@ Definition* BinaryIntegerOpInstr::Canonicalize(FlowGraph* flow_graph) {
21622162
break;
21632163

21642164
case Token::kSHL: {
2165-
const intptr_t kMaxShift =
2166-
SignificantRepresentationBits(representation()) - 1;
2165+
const intptr_t kMaxShift = RepresentationBits(representation()) - 1;
21672166
if (rhs == 0) {
21682167
return left()->definition();
21692168
} else if ((rhs < 0) || (rhs >= kMaxShift)) {

0 commit comments

Comments
 (0)