Change dart2js' Random to honor the seed.

Change dart2js' Random to honor the seed.

Simulate the same RNG as the VM when a seed is passed.

BUG= http://dartbug.com/15426
R=floitsch@google.com

Committed: https://code.google.com/p/dart/source/detail?r=30893

Committed: https://code.google.com/p/dart/source/detail?r=30894

[ngeoffray, 2013-12-04 11:22:15]

https://codereview.chromium.org/104083003/diff/1/sdk/lib/_internal/lib/math_p...
File sdk/lib/_internal/lib/math_patch.dart (right):

https://codereview.chromium.org/104083003/diff/1/sdk/lib/_internal/lib/math_p...
sdk/lib/_internal/lib/math_patch.dart:97: _hi = (seed - _lo) ~/ _POW2_32;
USe >> ?

https://codereview.chromium.org/104083003/diff/1/sdk/lib/_internal/lib/math_p...
sdk/lib/_internal/lib/math_patch.dart:119: var tmpHiLo = tmpHi >> 0;
Add a comment on why you're doing >> 0.

https://codereview.chromium.org/104083003/diff/1/sdk/lib/_internal/lib/math_p...
sdk/lib/_internal/lib/math_patch.dart:122: var tmpLoLo = tmpLo >> 0;
ditto

https://codereview.chromium.org/104083003/diff/1/sdk/lib/_internal/lib/math_p...
sdk/lib/_internal/lib/math_patch.dart:127: _hi = (tmpLoHi + tmpHiHi + newLoHi)
>> 0;
ditto

https://codereview.chromium.org/104083003/diff/1/sdk/lib/_internal/lib/math_p...
sdk/lib/_internal/lib/math_patch.dart:142: var rnd32;
var -> int

https://codereview.chromium.org/104083003/diff/1/sdk/lib/_internal/lib/math_p...
sdk/lib/_internal/lib/math_patch.dart:143: var result;
var -> int.

https://codereview.chromium.org/104083003/diff/1/sdk/lib/_internal/lib/math_p...
sdk/lib/_internal/lib/math_patch.dart:154: var bits26 = _lo & ((1 << 26) - 1);
var -> int, or num?

[Lasse Reichstein Nielsen, 2013-12-04 13:48:14]

https://codereview.chromium.org/104083003/diff/1/sdk/lib/_internal/lib/math_p...
File sdk/lib/_internal/lib/math_patch.dart (right):

https://codereview.chromium.org/104083003/diff/1/sdk/lib/_internal/lib/math_p...
sdk/lib/_internal/lib/math_patch.dart:97: _hi = (seed - _lo) ~/ _POW2_32;
Doesn't work, it's for the bits above 32.

https://codereview.chromium.org/104083003/diff/1/sdk/lib/_internal/lib/math_p...
sdk/lib/_internal/lib/math_patch.dart:119: var tmpHiLo = tmpHi >> 0;
Done. It's an implicit toInt32.
I should probably do & 0xFFFFFFFF instead.

https://codereview.chromium.org/104083003/diff/1/sdk/lib/_internal/lib/math_p...
sdk/lib/_internal/lib/math_patch.dart:122: var tmpLoLo = tmpLo >> 0;
Same pattern as above, so I'll just comment it once.

https://codereview.chromium.org/104083003/diff/1/sdk/lib/_internal/lib/math_p...
sdk/lib/_internal/lib/math_patch.dart:127: _hi = (tmpLoHi + tmpHiHi + newLoHi)
>> 0;
Same.

https://codereview.chromium.org/104083003/diff/1/sdk/lib/_internal/lib/math_p...
sdk/lib/_internal/lib/math_patch.dart:142: var rnd32;
On 2013/12/04 11:22:15, ngeoffray wrote:
> var -> int

Done.

https://codereview.chromium.org/104083003/diff/1/sdk/lib/_internal/lib/math_p...
sdk/lib/_internal/lib/math_patch.dart:143: var result;
On 2013/12/04 11:22:15, ngeoffray wrote:
> var -> int.

Done.

https://codereview.chromium.org/104083003/diff/1/sdk/lib/_internal/lib/math_p...
sdk/lib/_internal/lib/math_patch.dart:154: var bits26 = _lo & ((1 << 26) - 1);
On 2013/12/04 11:22:15, ngeoffray wrote:
> var -> int, or num?

Done.

[floitsch, 2013-12-04 15:18:44]

LGTM.

https://codereview.chromium.org/104083003/diff/60001/sdk/lib/_internal/lib/ma...
File sdk/lib/_internal/lib/math_patch.dart (right):

https://codereview.chromium.org/104083003/diff/60001/sdk/lib/_internal/lib/ma...
sdk/lib/_internal/lib/math_patch.dart:109: // The constant A is selected from
"Numerical Recipes 3rd Edition" p.348 B1.
What constant "A" ?

https://codereview.chromium.org/104083003/diff/60001/sdk/lib/_internal/lib/ma...
sdk/lib/_internal/lib/math_patch.dart:112: //   var state = ((0xFFFFDA61 *
(_state[kSTATE_LO])) +
remove parenthesis around _state[kSTATE_LO].
remove parenthesis around *. The default-associations are well known, and this
way one doesn't need to count parenthesis.

https://codereview.chromium.org/104083003/diff/60001/sdk/lib/_internal/lib/ma...
sdk/lib/_internal/lib/math_patch.dart:124: int tmpLoHi = (tmpLo - tmpLoLo) ~/
_POW2_32;
Probably not worth doing here, but if I'm not wrong we could avoid the ~/
division here if we do it together with the newLoHi computation.

https://codereview.chromium.org/104083003/diff/60001/sdk/lib/_internal/lib/ma...
sdk/lib/_internal/lib/math_patch.dart:149: result = rnd32 % max;
rnd32.remainder(max)

[Lasse Reichstein Nielsen, 2013-12-05 13:06:13]

https://codereview.chromium.org/104083003/diff/60001/sdk/lib/_internal/lib/ma...
File sdk/lib/_internal/lib/math_patch.dart (right):

https://codereview.chromium.org/104083003/diff/60001/sdk/lib/_internal/lib/ma...
sdk/lib/_internal/lib/math_patch.dart:109: // The constant A is selected from
"Numerical Recipes 3rd Edition" p.348 B1.
Ack. A = 0xFFFFDA61. Will put it back into the comment.

https://codereview.chromium.org/104083003/diff/60001/sdk/lib/_internal/lib/ma...
sdk/lib/_internal/lib/math_patch.dart:112: //   var state = ((0xFFFFDA61 *
(_state[kSTATE_LO])) +
On 2013/12/04 15:18:44, floitsch wrote:
> remove parenthesis around _state[kSTATE_LO].
> remove parenthesis around *. The default-associations are well known, and this
> way one doesn't need to count parenthesis.

Done, and rewriting _state[kSTATE_LO] to _lo.

https://codereview.chromium.org/104083003/diff/60001/sdk/lib/_internal/lib/ma...
sdk/lib/_internal/lib/math_patch.dart:120: int tmpHiLo = tmpHi & MASK32;        
    // Get the lower 32 bits.
This operation can be optimized better by dart2js.
It becomes: 
  (tempHi & 0xFFFFFFFF) >>> 0
and it could just be:
   tempHi >>> 0

I use the & 0xFFFFFFFF as "toUint32" operation because it is correct in Dart as
well as in dart2js. I could just use "| 0", but that would not be correct Dart.

https://codereview.chromium.org/104083003/diff/60001/sdk/lib/_internal/lib/ma...
sdk/lib/_internal/lib/math_patch.dart:124: int tmpLoHi = (tmpLo - tmpLoLo) ~/
_POW2_32;
You are right, there three values that are divided by 2^32 are later added. We
could add before dividing. I'll just do that.

https://codereview.chromium.org/104083003/diff/60001/sdk/lib/_internal/lib/ma...
sdk/lib/_internal/lib/math_patch.dart:149: result = rnd32 % max;
Better code generated, yey!

Could ofcourse still be smaller using JS("int", "# % #", rnd32, max), by
avoiding the null check on rnd32.

The generated code is:
    rnd32.toString;  // I assume this is a null test.
    result = rnd32 % max;   //
The null test might not be necessary because the next operation will also fail
on a null (but that's just an optimization for dart2js).

[Lasse Reichstein Nielsen, 2013-12-05 13:07:01]

Committed patchset #5 manually as r30893 (presubmit successful).

[Lasse Reichstein Nielsen, 2013-12-05 13:19:21]

Committed patchset #6 manually as r30894 (presubmit successful).

[floitsch, 2013-12-05 14:25:42]

https://codereview.chromium.org/104083003/diff/60001/sdk/lib/_internal/lib/ma...
File sdk/lib/_internal/lib/math_patch.dart (right):

https://codereview.chromium.org/104083003/diff/60001/sdk/lib/_internal/lib/ma...
sdk/lib/_internal/lib/math_patch.dart:149: result = rnd32 % max;
On 2013/12/05 13:06:14, Lasse Reichstein Nielsen wrote:
> Better code generated, yey!
> 
> Could ofcourse still be smaller using JS("int", "# % #", rnd32, max), by
> avoiding the null check on rnd32.
> 
> The generated code is:
>     rnd32.toString;  // I assume this is a null test.
>     result = rnd32 % max;   //
> The null test might not be necessary because the next operation will also fail
> on a null (but that's just an optimization for dart2js).

null & x is a valid expression in JS...