On 2014/04/03 16:02:45, oetuaho-nv wrote:
Thanks for the quick review, one comment inline.
I can look into this alternative solution and submit a new version once I
have
code and test results ready, and submit the assembler bug fix separately.
https://codereview.chromium.org/222403002/diff/1/src/arm/macro-assembler-arm.cc
File src/arm/macro-assembler-arm.cc (left):
https://codereview.chromium.org/222403002/diff/1/src/arm/macro-assembler-arm.cc#oldcode3819
src/arm/macro-assembler-arm.cc:3819: // Set rounding mode to round to the
nearest integer by clearing bits[23:22].
On 2014/04/03 09:22:48, jbramley wrote:
> Shouldn't we be in the right rounding mode already? ECMAScript maths
operations
> are supposed to be done in round-to-nearest mode, and this is also the
default
> FPSCR setting. If we've changed it explicitly somewhere else (for
whatever
> reason), we're probably not doing normal ECMAScript maths properly.
>
> If I'm correct about that, the whole thing collapses down:
>
> // Handle inputs >= 255 (including +infinity).
> mov(result_reg, 255);
> Vmov(double_scratch, 255.0, result_reg);
> VFPCompareAndSetFlags(input_reg, double_scratch);
> b(ge, &done);
>
> // All other inputs will clamp to the range [0-255]: NaN and -infinity
both
> produce 0.
> vcvt_u32_f64(double_scratch.low(), input_reg, kFPSCRRounding);
> Vmov(result_reg, double_scratch.low());
>
> This is more-or-less equivalent to what we did in ClampDoubleToUint8 in
> src/arm64/macro-assembler-arm64.cc, though the available instructions
make
it
> much simpler in A64.
This would be an excellent solution, but it seems the FPSCR state can be
wrong
when entering this function. VFPEnsureFPSCRState doesn't currently set the
rounding mode, and I suppose it can be messed with by outside code. But if
setting the FPSCR state in VFPEnsureFPSCRState will be enough, this
solution
could work.
The rounding mode should be round to nearest or other arithmetic operations
are
potentially incorrect. You could add a check in debug mode to
VFPEnsureFPSCRState to check the rounding mode is round to nearest.
https://codereview.chromium.org/222403002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
