Improve performance of Math.min and Math.max for the case of two arguments.

Improve performance of Math.min and Math.max for the case of two arguments.

BUG=v8:1325
TEST=

Committed: http://code.google.com/p/v8/source/detail?r=10321

[Yang, 2012-01-02 16:11:01]

Please take a look.

Math.min and Math.max are almost always used with two arguments only.  By
handling this case separately and not calling built-in functions to get the
arguments, we gain some performance.

[Lasse Reichstein, 2012-01-03 06:17:45]

drive-by-comment

http://codereview.chromium.org/9065008/diff/1/src/math.js
File src/math.js (right):

http://codereview.chromium.org/9065008/diff/1/src/math.js#newcode130
src/math.js:130: return (arg1 === 0 && !%_IsSmi(arg1) && 1 / arg1 < 0) ? arg2 :
arg1;
You can drop the 1/arg1 test, and actually also the '=== 0' test.
How about just:
  return %_IsSmi(arg1) ? arg1 : arg2;
You already know that the values are equal, so just pick the best one (the smi),
which also works out to what you want for 0/-0.

http://codereview.chromium.org/9065008/diff/1/src/math.js#newcode163
src/math.js:163: return (arg1 === 0 && !%_IsSmi(arg1) && 1 / arg1 < 0) ? arg1 :
arg2;
Here you want to return the non-smi if the values are zero. Otherwise you'd
probably want the smi:
  return ((arg1 === 0) != (%_IsSmi(arg1))) ? arg1 : arg2;

[Lasse Reichstein, 2012-01-03 07:57:48]

http://codereview.chromium.org/9065008/diff/1/src/math.js
File src/math.js (right):

http://codereview.chromium.org/9065008/diff/1/src/math.js#newcode130
src/math.js:130: return (arg1 === 0 && !%_IsSmi(arg1) && 1 / arg1 < 0) ? arg2 :
arg1;
Ignore that. If you have +0 as a HeapNumber, that approach isn't enough, and you
do need to do the division.
We could consider creating a %_IsMinusZero predicate to avoid the arithmetic.

http://codereview.chromium.org/9065008/diff/1/src/math.js#newcode163
src/math.js:163: return (arg1 === 0 && !%_IsSmi(arg1) && 1 / arg1 < 0) ? arg1 :
arg2;
Same problem here if +0 and -0 both are non-smi, so ariginal code is correct.

[Yang, 2012-01-03 08:59:34]

I implemented %_IsMinusZero for non-crankshaft, and running without crankshaft
there seems to be no visible difference, so I guess this is not worth
implementing (there is a lot more work involved in crankshaft, too).

On 2012/01/03 07:57:48, Lasse Reichstein wrote:
> http://codereview.chromium.org/9065008/diff/1/src/math.js
> File src/math.js (right):
> 
> http://codereview.chromium.org/9065008/diff/1/src/math.js#newcode130
> src/math.js:130: return (arg1 === 0 && !%_IsSmi(arg1) && 1 / arg1 < 0) ? arg2
:
> arg1;
> Ignore that. If you have +0 as a HeapNumber, that approach isn't enough, and
you
> do need to do the division.
> We could consider creating a %_IsMinusZero predicate to avoid the arithmetic.
> 
> http://codereview.chromium.org/9065008/diff/1/src/math.js#newcode163
> src/math.js:163: return (arg1 === 0 && !%_IsSmi(arg1) && 1 / arg1 < 0) ? arg1
:
> arg2;
> Same problem here if +0 and -0 both are non-smi, so ariginal code is correct.

[fschneider, 2012-01-03 09:52:06]

LGTM.

http://codereview.chromium.org/9065008/diff/1/src/math.js
File src/math.js (right):

http://codereview.chromium.org/9065008/diff/1/src/math.js#newcode127
src/math.js:127: if (arg1 == arg2) {
It does not make a difference for numbers, but I'd consistently use either ===
or ==.