[cleanup] Better implementation of SmiTagging<4>::IsValidSmi()

... which works properly when intptr_t is bigger than int and makes
MSVC happy about this.

Bug: v8:9183
Change-Id: I224eff00c1cbcb9a8c9f16eadaec078db7cf16db
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1601511
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61405}

include/v8-internal.h

@@ -48,28 +48,32 @@ const intptr_t kSmiTagMask = (1 << kSmiTagSize) - 1;
 template <size_t tagged_ptr_size>
 struct SmiTagging;
 
+constexpr intptr_t kIntptrAllBitsSet = intptr_t{-1};
+constexpr uintptr_t kUintptrAllBitsSet =
+    static_cast<uintptr_t>(kIntptrAllBitsSet);
+
 // Smi constants for systems where tagged pointer is a 32-bit value.
 template <>
 struct SmiTagging<4> {
   enum { kSmiShiftSize = 0, kSmiValueSize = 31 };
+
+  static constexpr intptr_t kSmiMinValue =
+      static_cast<intptr_t>(kUintptrAllBitsSet << (kSmiValueSize - 1));
+  static constexpr intptr_t kSmiMaxValue = -(kSmiMinValue + 1);
+
   V8_INLINE static int SmiToInt(const internal::Address value) {
     int shift_bits = kSmiTagSize + kSmiShiftSize;
     // Shift down (requires >> to be sign extending).
     return static_cast<int>(static_cast<intptr_t>(value)) >> shift_bits;
   }
   V8_INLINE static constexpr bool IsValidSmi(intptr_t value) {
-    // To be representable as an tagged small integer, the two
+    // Is value in range [kSmiMinValue, kSmiMaxValue].
-    // most-significant bits of 'value' must be either 00 or 11 due to
+    // Use unsigned operations in order to avoid undefined behaviour in case of
-    // sign-extension. To check this we add 01 to the two
+    // signed integer overflow.
-    // most-significant bits, and check if the most-significant bit is 0.
+    return (static_cast<uintptr_t>(value) -
-    //
+            static_cast<uintptr_t>(kSmiMinValue)) <=
-    // CAUTION: The original code below:
+           (static_cast<uintptr_t>(kSmiMaxValue) -
-    // bool result = ((value + 0x40000000) & 0x80000000) == 0;
+            static_cast<uintptr_t>(kSmiMinValue));
-    // may lead to incorrect results according to the C language spec, and
-    // in fact doesn't work correctly with gcc4.1.1 in some cases: The
-    // compiler may produce undefined results in case of signed integer
-    // overflow. The computation must be done w/ unsigned ints.
-    return static_cast<uintptr_t>(value) + 0x40000000U < 0x80000000U;
   }
 };
 
@@ -77,6 +81,11 @@ struct SmiTagging<4> {
 template <>
 struct SmiTagging<8> {
   enum { kSmiShiftSize = 31, kSmiValueSize = 32 };
+
+  static constexpr intptr_t kSmiMinValue =
+      static_cast<intptr_t>(kUintptrAllBitsSet << (kSmiValueSize - 1));
+  static constexpr intptr_t kSmiMaxValue = -(kSmiMinValue + 1);
+
   V8_INLINE static int SmiToInt(const internal::Address value) {
     int shift_bits = kSmiTagSize + kSmiShiftSize;
     // Shift down and throw away top 32 bits.
@@ -98,15 +107,15 @@ const int kApiTaggedSize = kApiSystemPointerSize;
 #endif
 
 #ifdef V8_31BIT_SMIS_ON_64BIT_ARCH
-typedef SmiTagging<kApiInt32Size> PlatformSmiTagging;
+using PlatformSmiTagging = SmiTagging<kApiInt32Size>;
 #else
-typedef SmiTagging<kApiTaggedSize> PlatformSmiTagging;
+using PlatformSmiTagging = SmiTagging<kApiTaggedSize>;
 #endif
 
 const int kSmiShiftSize = PlatformSmiTagging::kSmiShiftSize;
 const int kSmiValueSize = PlatformSmiTagging::kSmiValueSize;
-const int kSmiMinValue = (static_cast<unsigned int>(-1)) << (kSmiValueSize - 1);
+const int kSmiMinValue = static_cast<int>(PlatformSmiTagging::kSmiMinValue);
-const int kSmiMaxValue = -(kSmiMinValue + 1);
+const int kSmiMaxValue = static_cast<int>(PlatformSmiTagging::kSmiMaxValue);
 constexpr bool SmiValuesAre31Bits() { return kSmiValueSize == 31; }
 constexpr bool SmiValuesAre32Bits() { return kSmiValueSize == 32; }
 

src/globals.h

@@ -162,7 +162,6 @@ constexpr size_t kMaxWasmCodeMemory = kMaxWasmCodeMB * MB;
 constexpr int kSystemPointerSizeLog2 = 3;
 constexpr intptr_t kIntptrSignBit =
     static_cast<intptr_t>(uintptr_t{0x8000000000000000});
-constexpr uintptr_t kUintptrAllBitsSet = uintptr_t{0xFFFFFFFFFFFFFFFF};
 constexpr bool kRequiresCodeRange = true;
 #if V8_HOST_ARCH_PPC && V8_TARGET_ARCH_PPC && V8_OS_LINUX
 constexpr size_t kMaximalCodeRangeSize = 512 * MB;
@@ -184,7 +183,6 @@ constexpr size_t kReservedCodeRangePages = 0;
 #else
 constexpr int kSystemPointerSizeLog2 = 2;
 constexpr intptr_t kIntptrSignBit = 0x80000000;
-constexpr uintptr_t kUintptrAllBitsSet = 0xFFFFFFFFu;
 #if V8_HOST_ARCH_PPC && V8_TARGET_ARCH_PPC && V8_OS_LINUX
 constexpr bool kRequiresCodeRange = false;
 constexpr size_t kMaximalCodeRangeSize = 0 * MB;