Bring back the fisttp instruction on machines with SSE3, but check the...

Bring back the fisttp instruction on machines with SSE3, but check the
input so we don't have to check the exception flags afterwards.

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

[Lasse Reichstein, 2009-12-20 11:32:11]

Drive by review, but LGTM.

http://codereview.chromium.org/509001/diff/1/2
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/509001/diff/1/2#newcode7446
src/ia32/codegen-ia32.cc:7446: __ j(greater, conversion_failure);
Naming: If it's equal to too_big_exponent, it should be too big. 
Increase to_big_exponent by one, and use greater_equal.

Also, it only bails out for too large exponents, not too small. If the input is
a denormal, I have a hunch that the fisttp operation will set the denormal
exception flag, which will need to be cleared.
Maybe check that the exponent is in the range 0..63 (biased), perhaps by first
checking if the exponent is less than zero and returning zero in that case.

http://codereview.chromium.org/509001/diff/1/2#newcode7453
src/ia32/codegen-ia32.cc:7453: __ mov(ecx, Operand(esp, 0));  // Load low word
of answer into ecx.
pop(ecx) would be shorter (and then only drop one element below).

[sra1, 2009-12-20 19:42:07]

http://codereview.chromium.org/509001/diff/1/2
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/509001/diff/1/2#newcode7446
src/ia32/codegen-ia32.cc:7446: __ j(greater, conversion_failure);
On 2009/12/20 11:32:12, Lasse Reichstein wrote:
> Naming: If it's equal to too_big_exponent, it should be too big. 
> Increase to_big_exponent by one, and use greater_equal.
> 
> Also, it only bails out for too large exponents, not too small. If the input
is
> a denormal, I have a hunch that the fisttp operation will set the denormal
> exception flag, which will need to be cleared.
> Maybe check that the exponent is in the range 0..63 (biased), perhaps by first
> checking if the exponent is less than zero and returning zero in that case.

Obviously, additional unit tests would help demonstrate this is solid.  How
could you test that there was not an uncleared exception?  I don't recall seeing
any denormals in the unit tests.

I don't see any tests in smi-ops.js for fp arguments to xor that have
interesting fp values, like  anyof(3, 3.01, 3.99) ^ anyof(-4, -4.01, -4.99) ==
-1

http://codereview.chromium.org/509001/diff/1/2#newcode7453
src/ia32/codegen-ia32.cc:7453: __ mov(ecx, Operand(esp, 0));  // Load low word
of answer into ecx.
On 2009/12/20 11:32:12, Lasse Reichstein wrote:
> pop(ecx) would be shorter (and then only drop one element below).

It is shorter but probably not faster since there is a data dependency on esp
between pop and the add esp, 4.  As it stands, mov and add are independent.

[Erik Corry, 2009-12-21 10:14:13]

http://codereview.chromium.org/509001/diff/1004/1005
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/509001/diff/1004/1005#newcode7452
src/ia32/codegen-ia32.cc:7452: __ fnclex();
I added some testing here to ensure that there were no corner cases that were
causing uncaught exceptions.  This will be removed again.  Together with the CPU
instruction set doc and the new smi-ops.js test I am confident that this is
sound.  On the other hand I had to insert the fnclex instruction because
something in the system is leaving errors in the FPU processor status word
somewhere else in the VM.  I don't think this is a problem though it is worth
bearing in mind for future code that uses fnstsw_ax.