On 2014/04/03 18:29:20, Rodolph Perfetta wrote:
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.
I can't find any use of vmsr that sets the rounding mode to anything else.
A few
tests hit it, but the rounding mode is correct in every case. Under what
conditions can FPSCR be wrong here?
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.
That sounds like a good idea.
https://codereview.chromium.org/222403002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
