Revert of Implement xorshift128+ for Math.random. (patchset #6 id:100001 of...

Revert of Implement xorshift128+ for Math.random. (patchset #6 id:100001 of https://codereview.chromium.org/1464303002/ )

Reason for revert:
Test failure: http://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20gc%20stress/builds/491/steps/Mjsunit/logs/reflect-own-keys

Original issue's description:
> Implement xorshift128+ for Math.random.
>
> BUG=v8:4566
> LOG=N
>
> Committed: https://crrev.com/2755c5a1b1cf7fc4c5c614378e5231636e6dcff5
> Cr-Commit-Position: refs/heads/master@{#32200}

TBR=bmeurer@chromium.org,jkummerow@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:4566

Review URL: https://codereview.chromium.org/1475493003

Cr-Commit-Position: refs/heads/master@{#32202}

src/base/utils/random-number-generator.cc

@@ -97,14 +97,15 @@ int RandomNumberGenerator::NextInt(int max) {
 
 
 double RandomNumberGenerator::NextDouble() {
-  XorShift128(&state0_, &state1_);
-  return ToDouble(state0_, state1_);
+  return ((static_cast<int64_t>(Next(26)) << 27) + Next(27)) /
+      static_cast<double>(static_cast<int64_t>(1) << 53);
 }
 
 
 int64_t RandomNumberGenerator::NextInt64() {
-  XorShift128(&state0_, &state1_);
-  return bit_cast<int64_t>(state0_ + state1_);
+  uint64_t lo = bit_cast<unsigned>(Next(32));
+  uint64_t hi = bit_cast<unsigned>(Next(32));
+  return lo | (hi << 32);
 }
 
 
@@ -118,25 +119,21 @@ void RandomNumberGenerator::NextBytes(void* buffer, size_t buflen) {
 int RandomNumberGenerator::Next(int bits) {
   DCHECK_LT(0, bits);
   DCHECK_GE(32, bits);
-  XorShift128(&state0_, &state1_);
-  return static_cast<int>((state0_ + state1_) >> (64 - bits));
+  // Do unsigned multiplication, which has the intended modulo semantics, while
+  // signed multiplication would expose undefined behavior.
+  uint64_t product = static_cast<uint64_t>(seed_) * kMultiplier;
+  // Assigning a uint64_t to an int64_t is implementation defined, but this
+  // should be OK. Use a static_cast to explicitly state that we know what we're
+  // doing. (Famous last words...)
+  int64_t seed = static_cast<int64_t>((product + kAddend) & kMask);
+  seed_ = seed;
+  return static_cast<int>(seed >> (48 - bits));
 }
 
 
 void RandomNumberGenerator::SetSeed(int64_t seed) {
   initial_seed_ = seed;
-  state0_ = MurmurHash3(bit_cast<uint64_t>(seed));
-  state1_ = MurmurHash3(state0_);
-}
-
-
-uint64_t RandomNumberGenerator::MurmurHash3(uint64_t h) {
-  h ^= h >> 33;
-  h *= V8_UINT64_C(0xFF51AFD7ED558CCD);
-  h ^= h >> 33;
-  h *= V8_UINT64_C(0xC4CEB9FE1A85EC53);
-  h ^= h >> 33;
-  return h;
+  seed_ = (seed ^ kMultiplier) & kMask;
 }
 
 }  // namespace base

src/base/utils/random-number-generator.h

@@ -12,16 +12,10 @@ namespace base {
 
 // -----------------------------------------------------------------------------
 // RandomNumberGenerator
-
-// This class is used to generate a stream of pseudo-random numbers. The class
-// uses a 64-bit seed, which is passed through MurmurHash3 to create two 64-bit
-// state values. This pair of state values is then used in xorshift128+.
-// The resulting stream of pseudo-random numbers has a period length of 2^128-1.
-// See Marsaglia: http://www.jstatsoft.org/v08/i14/paper
-// And Vigna: http://vigna.di.unimi.it/ftp/papers/xorshiftplus.pdf
-// NOTE: Any changes to the algorithm must be tested against TestU01.
-//       Please find instructions for this in the internal repository.
-
+//
+// This class is used to generate a stream of pseudorandom numbers. The class
+// uses a 48-bit seed, which is modified using a linear congruential formula.
+// (See Donald Knuth, The Art of Computer Programming, Volume 3, Section 3.2.1.)
 // If two instances of RandomNumberGenerator are created with the same seed, and
 // the same sequence of method calls is made for each, they will generate and
 // return identical sequences of numbers.
@@ -89,27 +83,6 @@ class RandomNumberGenerator final {
 
   int64_t initial_seed() const { return initial_seed_; }
 
-  // Static and exposed for external use.
-  static inline double ToDouble(uint64_t state0, uint64_t state1) {
-    // Exponent for double values for [1.0 .. 2.0)
-    static const uint64_t kExponentBits = V8_UINT64_C(0x3FF0000000000000);
-    static const uint64_t kMantissaMask = V8_UINT64_C(0x000FFFFFFFFFFFFF);
-    uint64_t random = ((state0 + state1) & kMantissaMask) | kExponentBits;
-    return bit_cast<double>(random) - 1;
-  }
-
-  // Static and exposed for external use.
-  static inline void XorShift128(uint64_t* state0, uint64_t* state1) {
-    uint64_t s1 = *state0;
-    uint64_t s0 = *state1;
-    *state0 = s0;
-    s1 ^= s1 << 23;
-    s1 ^= s1 >> 17;
-    s1 ^= s0;
-    s1 ^= s0 >> 26;
-    *state1 = s1;
-  }
-
  private:
   static const int64_t kMultiplier = V8_2PART_UINT64_C(0x5, deece66d);
   static const int64_t kAddend = 0xb;
@@ -117,11 +90,8 @@ class RandomNumberGenerator final {
 
   int Next(int bits) WARN_UNUSED_RESULT;
 
-  static uint64_t MurmurHash3(uint64_t);
-
   int64_t initial_seed_;
-  uint64_t state0_;
-  uint64_t state1_;
+  int64_t seed_;
 };
 
 }  // namespace base

src/bootstrapper.cc

@@ -1729,6 +1729,24 @@ static Handle<JSObject> ResolveBuiltinIdHolder(Handle<Context> native_context,
 }
 
 
+template <typename Data>
+Handle<JSTypedArray> CreateTypedArray(Isolate* isolate, ExternalArrayType type,
+                                      size_t num_elements, Data** data) {
+  size_t byte_length = num_elements * sizeof(**data);
+  Handle<JSArrayBuffer> buffer =
+      isolate->factory()->NewJSArrayBuffer(SharedFlag::kNotShared, TENURED);
+  bool is_external = (*data != nullptr);
+  if (!is_external) {
+    *data = reinterpret_cast<Data*>(
+        isolate->array_buffer_allocator()->Allocate(byte_length));
+  }
+  JSArrayBuffer::Setup(buffer, isolate, is_external, *data, byte_length,
+                       SharedFlag::kNotShared);
+  return isolate->factory()->NewJSTypedArray(type, buffer, 0, num_elements,
+                                             TENURED);
+}
+
+
 void Genesis::ConfigureUtilsObject(ContextType context_type) {
   switch (context_type) {
     // We still need the utils object to find debug functions.

src/js/math.js

@@ -10,19 +10,18 @@
 // -------------------------------------------------------------------
 // Imports
 
-define kRandomBatchSize = 64;
-// The first two slots are reserved to persist PRNG state.
-define kRandomNumberStart = 2;
-
-var GlobalFloat64Array = global.Float64Array;
 var GlobalMath = global.Math;
 var GlobalObject = global.Object;
 var InternalArray = utils.InternalArray;
 var NaN = %GetRootNaN();
-var nextRandomIndex = kRandomBatchSize;
-var randomNumbers = UNDEFINED;
+var rngstate = { a: 1, b: 2, c: 3, d: 4 };
 var toStringTagSymbol = utils.ImportNow("to_string_tag_symbol");
 
+utils.InitializeRNG = function() {
+  var state = %InitializeRNG();
+  rngstate = { a: state[0], b: state[1], c: state[2], d: state[3] };
+};
+
 //-------------------------------------------------------------------
 
 // ECMA 262 - 15.8.2.1
@@ -136,19 +135,26 @@ function MathPowJS(x, y) {
 
 // ECMA 262 - 15.8.2.14
 function MathRandom() {
-  if (nextRandomIndex >= kRandomBatchSize) {
-    randomNumbers = %GenerateRandomNumbers(randomNumbers);
-    nextRandomIndex = kRandomNumberStart;
-  }
-  return randomNumbers[nextRandomIndex++];
+  var r0 = (MathImul(18030, rngstate.a) + rngstate.b) | 0;
+  var r1 = (MathImul(36969, rngstate.c) + rngstate.d) | 0;
+  rngstate.a = r0 & 0xFFFF;
+  rngstate.b = r0 >>> 16;
+  rngstate.c = r1 & 0xFFFF;
+  rngstate.d = r1 >>> 16;
+  var r = r0 ^ r1;
+  // Construct a double number 1.<32-bits of randomness> and subtract 1.
+  return %_ConstructDouble(0x3FF00000 | (r & 0x000FFFFF), r & 0xFFF00000) - 1;
 }
 
 function MathRandomRaw() {
-  if (nextRandomIndex >= kRandomBatchSize) {
-    randomNumbers = %GenerateRandomNumbers(randomNumbers);
-    nextRandomIndex = kRandomNumberStart;
-  }
-  return %_DoubleLo(randomNumbers[nextRandomIndex++]) & 0x3FFFFFFF;
+  var r0 = (MathImul(18030, rngstate.a) + rngstate.b) | 0;
+  var r1 = (MathImul(36969, rngstate.c) + rngstate.d) | 0;
+  rngstate.a = r0 & 0xFFFF;
+  rngstate.b = r0 >>> 16;
+  rngstate.c = r1 & 0xFFFF;
+  rngstate.d = r1 >>> 16;
+  var r = r0 ^ r1;
+  return r & 0x3FFFFFFF;
 }
 
 // ECMA 262 - 15.8.2.15

src/js/prologue.js

@@ -245,6 +245,8 @@ function PostExperimentals(utils) {
     imports_from_experimental(exports_container);
   }
 
+  utils.InitializeRNG();
+  utils.InitializeRNG = UNDEFINED;
   utils.CreateDoubleResultArray();
   utils.CreateDoubleResultArray = UNDEFINED;
 
@@ -260,6 +262,8 @@ function PostDebug(utils) {
     imports(exports_container);
   }
 
+  utils.InitializeRNG();
+  utils.InitializeRNG = UNDEFINED;
   utils.CreateDoubleResultArray();
   utils.CreateDoubleResultArray = UNDEFINED;
 
@@ -285,7 +289,7 @@ function InitializeBuiltinTypedArrays(utils, rng_state, rempio2result) {
 
 // -----------------------------------------------------------------------
 
-%OptimizeObjectForAddingMultipleProperties(utils, 14);
+%OptimizeObjectForAddingMultipleProperties(utils, 15);
 
 utils.Import = Import;
 utils.ImportNow = ImportNow;

src/runtime/runtime-maths.cc

@@ -247,48 +247,18 @@ RUNTIME_FUNCTION(Runtime_IsMinusZero) {
 }
 
 
-RUNTIME_FUNCTION(Runtime_GenerateRandomNumbers) {
+RUNTIME_FUNCTION(Runtime_InitializeRNG) {
   HandleScope scope(isolate);
-  DCHECK(args.length() == 1);
-  static const int kState0Offset = 0;
-  static const int kState1Offset = 1;
-  static const int kRandomBatchSize = 64;
-  CONVERT_ARG_HANDLE_CHECKED(Object, maybe_typed_array, 0);
-  Handle<JSTypedArray> typed_array;
-  // Allocate typed array if it does not yet exist.
-  if (maybe_typed_array->IsJSTypedArray()) {
-    typed_array = Handle<JSTypedArray>::cast(maybe_typed_array);
-  } else {
-    static const int kByteLength = kRandomBatchSize * kDoubleSize;
-    Handle<JSArrayBuffer> buffer =
-        isolate->factory()->NewJSArrayBuffer(SharedFlag::kNotShared, TENURED);
-    JSArrayBuffer::SetupAllocatingData(buffer, isolate, kByteLength, true,
-                                       SharedFlag::kNotShared);
-    typed_array = isolate->factory()->NewJSTypedArray(
-        kExternalFloat64Array, buffer, 0, kRandomBatchSize);
-  }
-
-  DisallowHeapAllocation no_gc;
-  double* array =
-      reinterpret_cast<double*>(typed_array->GetBuffer()->backing_store());
-  // Fetch existing state.
-  uint64_t state0 = double_to_uint64(array[kState0Offset]);
-  uint64_t state1 = double_to_uint64(array[kState1Offset]);
-  // Initialize state if not yet initialized.
-  while (state0 == 0 || state1 == 0) {
-    isolate->random_number_generator()->NextBytes(&state0, sizeof(state0));
-    isolate->random_number_generator()->NextBytes(&state1, sizeof(state1));
-  }
-  // Create random numbers.
-  for (int i = kState1Offset + 1; i < kRandomBatchSize; i++) {
-    // Generate random numbers using xorshift128+.
-    base::RandomNumberGenerator::XorShift128(&state0, &state1);
-    array[i] = base::RandomNumberGenerator::ToDouble(state0, state1);
-  }
-  // Persist current state.
-  array[kState0Offset] = uint64_to_double(state0);
-  array[kState1Offset] = uint64_to_double(state1);
-  return *typed_array;
+  DCHECK(args.length() == 0);
+  static const int kSize = 4;
+  Handle<FixedArray> array = isolate->factory()->NewFixedArray(kSize);
+  uint16_t seeds[kSize];
+  do {
+    isolate->random_number_generator()->NextBytes(seeds,
+                                                  kSize * sizeof(*seeds));
+  } while (!(seeds[0] && seeds[1] && seeds[2] && seeds[3]));
+  for (int i = 0; i < kSize; i++) array->set(i, Smi::FromInt(seeds[i]));
+  return *isolate->factory()->NewJSArrayWithElements(array);
 }
 }  // namespace internal
 }  // namespace v8

src/runtime/runtime.h

@@ -404,7 +404,7 @@ namespace internal {
   F(MathSqrt, 1, 1)                 \
   F(MathFround, 1, 1)               \
   F(IsMinusZero, 1, 1)              \
-  F(GenerateRandomNumbers, 1, 1)
+  F(InitializeRNG, 0, 1)
 
 
 #define FOR_EACH_INTRINSIC_NUMBERS(F)  \