dart2js js_runtime: faster code for floor() & ceil()

dart2js js_runtime: faster code for floor() & ceil()

Within the [-2^31, 2^31) range it is faster to truncate and correct at boundaries than call Math.ceil/Math.floor.

Each call to toInt() has been specialized to the calling operator (ceil, floor, ~/ etc).

This makes the conversion faster (values are already integral) and lets us put the name of the operation and original operands in conversion errors, e.g


(a / b).floor();  // b == 0
Unsupported operation: Infinity
-->
Unsupported operation: Infinity.floor()

a ~/ b;   // b == 0
Unsupported operation: Infinity
-->
Unsupported operation: Result of truncating division is Infinity: 132 ~/ 0


R=sigmund@google.com

Downside: an extra 400 bytes
Committed: https://github.com/dart-lang/sdk/commit/1eee3c7dc74aa22ef9b3a45b0fd821e67ac8da1d

[sra1, 2015-10-15 23:07:21]

Patchset #1 (id:1) has been deleted

[sra1, 2015-10-20 21:43:18]

Patchset #1 (id:20001) has been deleted

[sra1, 2015-10-21 01:59:59]

Description was changed from

==========
dart2js js_runtime: faster code for floor() & ceil()

Within the [-2^31, 2^31) range it is faster to truncate and correct at
boundaries than call Math.ceil/Math.floor.

Each call to toInt() has been specialized to the calling operator (ceil, floor,
~/ etc).

This makes the conversion faster (values are already integral) and lets us put
the name of the operation and operands in conversion errors, e.g


(a / b).floor();  // b == 0
Unsupported operation: Infinity
-->
Unsupported operation: Infinity.floor()

a ~/ b;   // b == 0
Unsupported operation: Infinity
-->
Unsupported operation: Result of truncating division is Infinity: 132 ~/ 0
==========

to

==========
dart2js js_runtime: faster code for floor() & ceil()

Within the [-2^31, 2^31) range it is faster to truncate and correct at
boundaries than call Math.ceil/Math.floor.

Each call to toInt() has been specialized to the calling operator (ceil, floor,
~/ etc).

This makes the conversion faster (values are already integral) and lets us put
the name of the operation and operands in conversion errors, e.g


(a / b).floor();  // b == 0
Unsupported operation: Infinity
-->
Unsupported operation: Infinity.floor()

a ~/ b;   // b == 0
Unsupported operation: Infinity
-->
Unsupported operation: Result of truncating division is Infinity: 132 ~/ 0


Downside: an extra 400 bytes
==========

[sra1, 2015-10-28 21:21:32]

Description was changed from

==========
dart2js js_runtime: faster code for floor() & ceil()

Within the [-2^31, 2^31) range it is faster to truncate and correct at
boundaries than call Math.ceil/Math.floor.

Each call to toInt() has been specialized to the calling operator (ceil, floor,
~/ etc).

This makes the conversion faster (values are already integral) and lets us put
the name of the operation and operands in conversion errors, e.g


(a / b).floor();  // b == 0
Unsupported operation: Infinity
-->
Unsupported operation: Infinity.floor()

a ~/ b;   // b == 0
Unsupported operation: Infinity
-->
Unsupported operation: Result of truncating division is Infinity: 132 ~/ 0


Downside: an extra 400 bytes
==========

to

==========
dart2js js_runtime: faster code for floor() & ceil()

Within the [-2^31, 2^31) range it is faster to truncate and correct at
boundaries than call Math.ceil/Math.floor.

Each call to toInt() has been specialized to the calling operator (ceil, floor,
~/ etc).

This makes the conversion faster (values are already integral) and lets us put
the name of the operation and original operands in conversion errors, e.g


(a / b).floor();  // b == 0
Unsupported operation: Infinity
-->
Unsupported operation: Infinity.floor()

a ~/ b;   // b == 0
Unsupported operation: Infinity
-->
Unsupported operation: Result of truncating division is Infinity: 132 ~/ 0


Downside: an extra 400 bytes
==========

[sra1, 2015-11-06 03:55:30]

Patchset #1 (id:40001) has been deleted

[sra1, 2015-11-06 04:06:04]

Patchset #1 (id:60001) has been deleted

[sra1, 2015-11-06 04:12:26]

sra@google.com changed reviewers:
+ sigmund@google.com

[Siggi Cherem (dart-lang), 2015-11-06 17:37:01]

lgtm

https://codereview.chromium.org/1405113003/diff/80001/sdk/lib/_internal/js_ru...
File sdk/lib/_internal/js_runtime/lib/js_number.dart (right):

https://codereview.chromium.org/1405113003/diff/80001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/js_number.dart:110: }
Would it make the code smaller if we refactor the code slightly, for example:

if (this >= _MIN && this <= _MAX) {
  int truncated = ...;
  return this == truncated || this >= 0 ? truncated : truncated + 1;
}

https://codereview.chromium.org/1405113003/diff/80001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/js_number.dart:287: if (other >= 1 || other <
-1) {
any reason why not combine this with the if above? (or will ssa combine the if
conditions?)

if (_isInt32(this) && (other >= 1 || other < -1)) {
  ...
}

Also - couldn't we do:

if (other >= 1 || other < 1) { // no isInt32 check
  return _tdivFast(other);
}

(and get rid if the if-false above?

[sra1, 2015-11-06 21:52:45]

https://codereview.chromium.org/1405113003/diff/80001/sdk/lib/_internal/js_ru...
File sdk/lib/_internal/js_runtime/lib/js_number.dart (right):

https://codereview.chromium.org/1405113003/diff/80001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/js_number.dart:110: }
On 2015/11/06 17:37:01, Siggi Cherem (dart-lang) wrote:
> Would it make the code smaller if we refactor the code slightly, for example:
> 
> if (this >= _MIN && this <= _MAX) {
>   int truncated = ...;
>   return this == truncated || this >= 0 ? truncated : truncated + 1;
> }
> 

The suggestion has an extra test which is measurable in micro-benchmarks.

https://codereview.chromium.org/1405113003/diff/80001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/js_number.dart:287: if (other >= 1 || other <
-1) {
On 2015/11/06 17:37:01, Siggi Cherem (dart-lang) wrote:
> any reason why not combine this with the if above? (or will ssa combine the if
> conditions?)
> 
> if (_isInt32(this) && (other >= 1 || other < -1)) {
>   ...
> }

We get better type inference with two ifs.
Dart has a 'choke point' in the implicit boolean conversion.

> 
> Also - couldn't we do:
> 
> if (other >= 1 || other < 1) { // no isInt32 check
>   return _tdivFast(other);
> }
> 
> (and get rid if the if-false above?

After inlining it ends up bigger since there are now two calls to _tdivSlow.

[sra1, 2016-07-15 19:28:09]

Description was changed from

==========
dart2js js_runtime: faster code for floor() & ceil()

Within the [-2^31, 2^31) range it is faster to truncate and correct at
boundaries than call Math.ceil/Math.floor.

Each call to toInt() has been specialized to the calling operator (ceil, floor,
~/ etc).

This makes the conversion faster (values are already integral) and lets us put
the name of the operation and original operands in conversion errors, e.g


(a / b).floor();  // b == 0
Unsupported operation: Infinity
-->
Unsupported operation: Infinity.floor()

a ~/ b;   // b == 0
Unsupported operation: Infinity
-->
Unsupported operation: Result of truncating division is Infinity: 132 ~/ 0


Downside: an extra 400 bytes
==========

to

==========
dart2js js_runtime: faster code for floor() & ceil()

Within the [-2^31, 2^31) range it is faster to truncate and correct at
boundaries than call Math.ceil/Math.floor.

Each call to toInt() has been specialized to the calling operator (ceil, floor,
~/ etc).

This makes the conversion faster (values are already integral) and lets us put
the name of the operation and original operands in conversion errors, e.g


(a / b).floor();  // b == 0
Unsupported operation: Infinity
-->
Unsupported operation: Infinity.floor()

a ~/ b;   // b == 0
Unsupported operation: Infinity
-->
Unsupported operation: Result of truncating division is Infinity: 132 ~/ 0


R=sigmund@google.com

Downside: an extra 400 bytes
Committed:
https://github.com/dart-lang/sdk/commit/1eee3c7dc74aa22ef9b3a45b0fd821e67ac8da1d
==========

[sra1, 2016-07-15 19:28:10]

Committed patchset #4 (id:140001) manually as
1eee3c7dc74aa22ef9b3a45b0fd821e67ac8da1d (presubmit successful).