[VM] Reduce Smi size to 32 bit on 64 bit platforms

This reduces small tagged integers on 64 bit platforms from 63 bits to 31 bits plus one tag bit. This is a step on the way to compile-time-optional compressed pointers on 64 bit platforms. See more about this at go/dartvmlearnings This causes a slowdown for some uses of integers that don't fit in 31 signed bits, but because both x64 and ARM64 have unboxed 64 bit integers now the performance hit should not be too bad. This reapplies the change reviewed at https://dart-review.googlesource.com/c/sdk/+/46244 [email protected] Change-Id: I605c21506ec7d4c69fa7049bc419b3ee370685fc Reviewed-on: https://dart-review.googlesource.com/50202 Reviewed-by: Martin Kustermann <[email protected]> Commit-Queue: Erik Corry <[email protected]>
aam · Apr 9, 2018 · 0e9a77a · 0e9a77a
1 parent 34bfefd
commit 0e9a77a
Show file tree

Hide file tree

Showing 24 changed files with 949 additions and 566 deletions.
diff --git a/runtime/lib/compact_hash.dart b/runtime/lib/compact_hash.dart
@@ -78,19 +78,14 @@ abstract class _HashBase implements _HashVMBase {
   static const int _UNUSED_PAIR = 0;
   static const int _DELETED_PAIR = 1;
 
-  // On 32-bit, the top bits are wasted to avoid Mint allocation.
-  // TODO(koda): Reclaim the bits by making the compiler treat hash patterns
-  // as unsigned words.
+  // The top bits are wasted to avoid Mint allocation.
   static int _indexSizeToHashMask(int indexSize) {
     int indexBits = indexSize.bitLength - 2;
-    return internal.is64Bit
-        ? (1 << (32 - indexBits)) - 1
-        : (1 << (30 - indexBits)) - 1;
+    return (1 << (30 - indexBits)) - 1;
   }
 
   static int _hashPattern(int fullHash, int hashMask, int size) {
     final int maskedHash = fullHash & hashMask;
-    // TODO(koda): Consider keeping bit length and use left shift.
     return (maskedHash == 0) ? (size >> 1) : maskedHash * (size >> 1);
   }
 

diff --git a/runtime/lib/integers_patch.dart b/runtime/lib/integers_patch.dart
@@ -32,7 +32,7 @@ class int {
         return null; // Empty.
       }
     }
-    var smiLimit = is64Bit ? 18 : 9;
+    var smiLimit = 9;
     if ((last - ix) >= smiLimit) {
       return null; // May not fit into a Smi.
     }
@@ -117,7 +117,7 @@ class int {
 
   static int _parseRadix(
       String source, int radix, int start, int end, int sign) {
-    int tableIndex = (radix - 2) * 4 + (is64Bit ? 2 : 0);
+    int tableIndex = (radix - 2) * 2;
     int blockSize = _PARSE_LIMITS[tableIndex];
     int length = end - start;
     if (length <= blockSize) {
@@ -143,7 +143,7 @@ class int {
     int positiveOverflowLimit = 0;
     int negativeOverflowLimit = 0;
     if (_limitIntsTo64Bits) {
-      tableIndex = tableIndex << 1; // pre-multiply by 2 for simpler indexing
+      tableIndex = tableIndex << 1; // Pre-multiply by 2 for simpler indexing.
       positiveOverflowLimit = _int64OverflowLimits[tableIndex];
       if (positiveOverflowLimit == 0) {
         positiveOverflowLimit =
@@ -159,14 +159,10 @@ class int {
         if (result >= positiveOverflowLimit) {
           if ((result > positiveOverflowLimit) ||
               (smi > _int64OverflowLimits[tableIndex + 2])) {
-            // Although the unsigned overflow limits do not depend on the
-            // platform, the multiplier and block size, which are used to
-            // compute it, do.
-            int X = is64Bit ? 1 : 0;
             if (radix == 16 &&
-                !(result >= _int64UnsignedOverflowLimits[X] &&
-                    (result > _int64UnsignedOverflowLimits[X] ||
-                        smi > _int64UnsignedSmiOverflowLimits[X])) &&
+                !(result >= _int64UnsignedOverflowLimit &&
+                    (result > _int64UnsignedOverflowLimit ||
+                        smi > _int64UnsignedSmiOverflowLimit)) &&
                 blockEnd + blockSize > end) {
               return (result * multiplier) + smi;
             }
@@ -211,43 +207,42 @@ class int {
 
   // For each radix, 2-36, how many digits are guaranteed to fit in a smi,
   // and magnitude of such a block (radix ** digit-count).
-  // 32-bit limit/multiplier at (radix - 2)*4, 64-bit limit at (radix-2)*4+2
   static const _PARSE_LIMITS = const [
-    30, 1073741824, 62, 4611686018427387904, // radix: 2
-    18, 387420489, 39, 4052555153018976267,
-    15, 1073741824, 30, 1152921504606846976,
-    12, 244140625, 26, 1490116119384765625, //  radix: 5
-    11, 362797056, 23, 789730223053602816,
-    10, 282475249, 22, 3909821048582988049,
-    10, 1073741824, 20, 1152921504606846976,
-    9, 387420489, 19, 1350851717672992089,
-    9, 1000000000, 18, 1000000000000000000, //  radix: 10
-    8, 214358881, 17, 505447028499293771,
-    8, 429981696, 17, 2218611106740436992,
-    8, 815730721, 16, 665416609183179841,
-    7, 105413504, 16, 2177953337809371136,
-    7, 170859375, 15, 437893890380859375, //    radix: 15
-    7, 268435456, 15, 1152921504606846976,
-    7, 410338673, 15, 2862423051509815793,
-    7, 612220032, 14, 374813367582081024,
-    7, 893871739, 14, 799006685782884121,
-    6, 64000000, 14, 1638400000000000000, //    radix: 20
-    6, 85766121, 14, 3243919932521508681,
-    6, 113379904, 13, 282810057883082752,
-    6, 148035889, 13, 504036361936467383,
-    6, 191102976, 13, 876488338465357824,
-    6, 244140625, 13, 1490116119384765625, //   radix: 25
-    6, 308915776, 13, 2481152873203736576,
-    6, 387420489, 13, 4052555153018976267,
-    6, 481890304, 12, 232218265089212416,
-    6, 594823321, 12, 353814783205469041,
-    6, 729000000, 12, 531441000000000000, //    radix: 30
-    6, 887503681, 12, 787662783788549761,
-    6, 1073741824, 12, 1152921504606846976,
-    5, 39135393, 12, 1667889514952984961,
-    5, 45435424, 12, 2386420683693101056,
-    5, 52521875, 12, 3379220508056640625, //    radix: 35
-    5, 60466176, 11, 131621703842267136,
+    30, 1073741824, // radix: 2
+    18, 387420489,
+    15, 1073741824,
+    12, 244140625, //  radix: 5
+    11, 362797056,
+    10, 282475249,
+    10, 1073741824,
+    9, 387420489,
+    9, 1000000000, //  radix: 10
+    8, 214358881,
+    8, 429981696,
+    8, 815730721,
+    7, 105413504,
+    7, 170859375, //    radix: 15
+    7, 268435456,
+    7, 410338673,
+    7, 612220032,
+    7, 893871739,
+    6, 64000000, //    radix: 20
+    6, 85766121,
+    6, 113379904,
+    6, 148035889,
+    6, 191102976,
+    6, 244140625, //   radix: 25
+    6, 308915776,
+    6, 387420489,
+    6, 481890304,
+    6, 594823321,
+    6, 729000000, //    radix: 30
+    6, 887503681,
+    6, 1073741824,
+    5, 39135393,
+    5, 45435424,
+    5, 52521875, //    radix: 35
+    5, 60466176,
   ];
 
   /// Flag indicating if integers are limited by 64 bits
@@ -257,11 +252,8 @@ class int {
   static const _maxInt64 = 0x7fffffffffffffff;
   static const _minInt64 = -_maxInt64 - 1;
 
-  static const _int64UnsignedOverflowLimits = const [0xfffffffff, 0xf];
-  static const _int64UnsignedSmiOverflowLimits = const [
-    0xfffffff,
-    0xfffffffffffffff
-  ];
+  static const _int64UnsignedOverflowLimit = 0xfffffffff;
+  static const _int64UnsignedSmiOverflowLimit = 0xfffffff;
 
   /// In the `--limit-ints-to-64-bits` mode calculation of the expression
   ///

diff --git a/runtime/vm/compiler/assembler/assembler_arm64.h b/runtime/vm/compiler/assembler/assembler_arm64.h
@@ -1069,12 +1069,18 @@ class Assembler : public ValueObject {
     const Register crn = ConcreteRegister(rn);
     EmitFPIntCvtOp(SCVTFD, static_cast<Register>(vd), crn, kWord);
   }
-  void fcvtzds(Register rd, VRegister vn) {
+  void fcvtzdsx(Register rd, VRegister vn) {
     ASSERT(rd != R31);
     ASSERT(rd != CSP);
     const Register crd = ConcreteRegister(rd);
     EmitFPIntCvtOp(FCVTZDS, crd, static_cast<Register>(vn));
   }
+  void fcvtzdsw(Register rd, VRegister vn) {
+    ASSERT(rd != R31);
+    ASSERT(rd != CSP);
+    const Register crd = ConcreteRegister(rd);
+    EmitFPIntCvtOp(FCVTZDS, crd, static_cast<Register>(vn), kWord);
+  }
   void fmovdd(VRegister vd, VRegister vn) { EmitFPOneSourceOp(FMOVDD, vd, vn); }
   void fabsd(VRegister vd, VRegister vn) { EmitFPOneSourceOp(FABSD, vd, vn); }
   void fnegd(VRegister vd, VRegister vn) { EmitFPOneSourceOp(FNEGD, vd, vn); }
@@ -1365,9 +1371,17 @@ class Assembler : public ValueObject {
     LslImmediate(dst, src, kSmiTagSize);
   }
 
-  void BranchIfNotSmi(Register reg, Label* label) { tbnz(label, reg, kSmiTag); }
+  void BranchIfNotSmi(Register reg, Label* label) {
+    ASSERT(kSmiTagMask == 1);
+    ASSERT(kSmiTag == 0);
+    tbnz(label, reg, 0);
+  }
 
-  void BranchIfSmi(Register reg, Label* label) { tbz(label, reg, kSmiTag); }
+  void BranchIfSmi(Register reg, Label* label) {
+    ASSERT(kSmiTagMask == 1);
+    ASSERT(kSmiTag == 0);
+    tbz(label, reg, 0);
+  }
 
   void Branch(const StubEntry& stub_entry,
               Register pp,
@@ -1451,6 +1465,11 @@ class Assembler : public ValueObject {
     kValueCanBeSmi,
   };
 
+  enum CanBeHeapPointer {
+    kValueIsNotHeapPointer,
+    kValueCanBeHeapPointer,
+  };
+
   // Storing into an object.
   void StoreIntoObject(Register object,
                        const Address& dest,
@@ -1591,6 +1610,22 @@ class Assembler : public ValueObject {
                       Register tmp,
                       OperandSize sz);
 
+  void AssertSmiInRange(
+      Register object,
+      CanBeHeapPointer can_be_heap_pointer = kValueIsNotHeapPointer) {
+#if defined(DEBUG)
+    Label ok;
+    if (can_be_heap_pointer == kValueCanBeHeapPointer) {
+      BranchIfNotSmi(object, &ok);
+    }
+    cmp(object, Operand(object, SXTW, 0));
+    b(&ok, EQ);
+    Stop("Smi out of range");
+
+    Bind(&ok);
+#endif
+  }
+
  private:
   AssemblerBuffer buffer_;  // Contains position independent code.
   ObjectPoolWrapper object_pool_wrapper_;

diff --git a/runtime/vm/compiler/assembler/assembler_arm64_test.cc b/runtime/vm/compiler/assembler/assembler_arm64_test.cc
@@ -2576,17 +2576,73 @@ ASSEMBLER_TEST_RUN(FldrqFstrqPrePostIndex, test) {
   EXPECT_EQ(42.0, EXECUTE_TEST_CODE_DOUBLE(DoubleReturn, test->entry()));
 }
 
-ASSEMBLER_TEST_GENERATE(Fcvtzds, assembler) {
+ASSEMBLER_TEST_GENERATE(Fcvtzdsx, assembler) {
   __ LoadDImmediate(V0, 42.0);
-  __ fcvtzds(R0, V0);
+  __ fcvtzdsx(R0, V0);
   __ ret();
 }
 
-ASSEMBLER_TEST_RUN(Fcvtzds, test) {
+ASSEMBLER_TEST_RUN(Fcvtzdsx, test) {
   typedef int64_t (*Int64Return)() DART_UNUSED;
   EXPECT_EQ(42, EXECUTE_TEST_CODE_INT64(Int64Return, test->entry()));
 }
 
+ASSEMBLER_TEST_GENERATE(Fcvtzdsw, assembler) {
+  __ LoadDImmediate(V0, 42.0);
+  __ fcvtzdsw(R0, V0);
+  __ ret();
+}
+
+ASSEMBLER_TEST_RUN(Fcvtzdsw, test) {
+  typedef int64_t (*Int64Return)() DART_UNUSED;
+  EXPECT_EQ(42, EXECUTE_TEST_CODE_INT64(Int64Return, test->entry()));
+}
+
+ASSEMBLER_TEST_GENERATE(Fcvtzdsx_overflow, assembler) {
+  __ LoadDImmediate(V0, 1e20);
+  __ fcvtzdsx(R0, V0);
+  __ ret();
+}
+
+ASSEMBLER_TEST_RUN(Fcvtzdsx_overflow, test) {
+  typedef int64_t (*Int64Return)() DART_UNUSED;
+  EXPECT_EQ(kMaxInt64, EXECUTE_TEST_CODE_INT64(Int64Return, test->entry()));
+}
+
+ASSEMBLER_TEST_GENERATE(Fcvtzdsx_overflow_negative, assembler) {
+  __ LoadDImmediate(V0, -1e20);
+  __ fcvtzdsx(R0, V0);
+  __ ret();
+}
+
+ASSEMBLER_TEST_RUN(Fcvtzdsx_overflow_negative, test) {
+  typedef int64_t (*Int64Return)() DART_UNUSED;
+  EXPECT_EQ(kMinInt64, EXECUTE_TEST_CODE_INT64(Int64Return, test->entry()));
+}
+
+ASSEMBLER_TEST_GENERATE(Fcvtzdsw_overflow, assembler) {
+  __ LoadDImmediate(V0, 1e10);
+  __ fcvtzdsw(R0, V0);
+  __ ret();
+}
+
+ASSEMBLER_TEST_RUN(Fcvtzdsw_overflow, test) {
+  typedef int64_t (*Int64Return)() DART_UNUSED;
+  EXPECT_EQ(kMaxInt32, EXECUTE_TEST_CODE_INT64(Int64Return, test->entry()));
+}
+
+ASSEMBLER_TEST_GENERATE(Fcvtzdsw_overflow_negative, assembler) {
+  __ LoadDImmediate(V0, -1e10);
+  __ fcvtzdsw(R0, V0);
+  __ sxtw(R0, R0);
+  __ ret();
+}
+
+ASSEMBLER_TEST_RUN(Fcvtzdsw_overflow_negative, test) {
+  typedef int64_t (*Int64Return)() DART_UNUSED;
+  EXPECT_EQ(kMinInt32, EXECUTE_TEST_CODE_INT64(Int64Return, test->entry()));
+}
+
 ASSEMBLER_TEST_GENERATE(Scvtfdx, assembler) {
   __ LoadImmediate(R0, 42);
   __ scvtfdx(V0, R0);

diff --git a/runtime/vm/compiler/assembler/assembler_x64.h b/runtime/vm/compiler/assembler/assembler_x64.h
@@ -521,7 +521,10 @@ class Assembler : public ValueObject {
     return CompareImmediate(reg, Immediate(immediate));
   }
 
-  void testl(Register reg, const Immediate& imm) { testq(reg, imm); }
+  void testl(Register reg, const Immediate& imm) {
+    Immediate imm2(imm.value() & 0xffffffffll);
+    testq(reg, imm2);
+  }
   void testb(const Address& address, const Immediate& imm);
 
   void testq(Register reg, const Immediate& imm);
@@ -710,6 +713,11 @@ class Assembler : public ValueObject {
     kValueCanBeSmi,
   };
 
+  enum CanBeHeapPointer {
+    kValueIsNotHeapPointer,
+    kValueCanBeHeapPointer,
+  };
+
   // Destroys value.
   void StoreIntoObject(Register object,      // Object we are storing into.
                        const Address& dest,  // Where we are storing into.
@@ -939,6 +947,26 @@ class Assembler : public ValueObject {
                                            Register array,
                                            Register index);
 
+  void AssertSmiInRange(
+      Register object,
+      CanBeHeapPointer can_be_heap_pointer = kValueIsNotHeapPointer) {
+#if defined(DEBUG)
+    Register tmp = object == TMP ? TMP2 : TMP;
+    Label ok;
+    if (can_be_heap_pointer == kValueCanBeHeapPointer) {
+      testl(object, Immediate(kSmiTagMask));
+      ASSERT(kSmiTag == 0);
+      j(ZERO, &ok);
+    }
+    movsxd(tmp, object);
+    cmpq(tmp, object);
+    j(EQUAL, &ok);
+    Stop("Smi out of range");
+
+    Bind(&ok);
+#endif
+  }
+
   static Address VMTagAddress() {
     return Address(THR, Thread::vm_tag_offset());
   }

diff --git a/runtime/vm/compiler/backend/il.cc b/runtime/vm/compiler/backend/il.cc
@@ -1584,10 +1584,10 @@ bool BinaryIntegerOpInstr::RightIsPowerOfTwoConstant() const {
   return Utils::IsPowerOfTwo(Utils::Abs(int_value));
 }
 
-static intptr_t RepresentationBits(Representation r) {
+static intptr_t SignificantRepresentationBits(Representation r) {
   switch (r) {
     case kTagged:
-      return kBitsPerWord - 1;
+      return 31;
     case kUnboxedInt32:
     case kUnboxedUint32:
       return 32;
@@ -1601,7 +1601,7 @@ static intptr_t RepresentationBits(Representation r) {
 
 static int64_t RepresentationMask(Representation r) {
   return static_cast<int64_t>(static_cast<uint64_t>(-1) >>
-                              (64 - RepresentationBits(r)));
+                              (64 - SignificantRepresentationBits(r)));
 }
 
 static bool ToIntegerConstant(Value* value, int64_t* result) {
@@ -2162,7 +2162,8 @@ Definition* BinaryIntegerOpInstr::Canonicalize(FlowGraph* flow_graph) {
       break;
 
     case Token::kSHL: {
-      const intptr_t kMaxShift = RepresentationBits(representation()) - 1;
+      const intptr_t kMaxShift =
+          SignificantRepresentationBits(representation()) - 1;
       if (rhs == 0) {
         return left()->definition();
       } else if ((rhs < 0) || (rhs >= kMaxShift)) {