On 2014/09/18 15:46:16, titzer wrote:
On 2014/09/18 14:31:21, Weiliang wrote:
> On 2014/09/18 14:11:01, Weiliang wrote:
> >
>
https://codereview.chromium.org/582713002/diff/1/src/compiler/ia32/code-generator-ia32.cc
> > File src/compiler/ia32/code-generator-ia32.cc (right):
> >
> >
>
https://codereview.chromium.org/582713002/diff/1/src/compiler/ia32/code-generator-ia32.cc#newcode310
> > src/compiler/ia32/code-generator-ia32.cc:310: __ j(not_carry, &done,
> > Label::kNear);
> > On 2014/09/18 14:03:06, titzer wrote:
> > > What does this extra condition here check?
> > >
> > > Note that this the implementation of ChangeFloat64ToUint32, which
explicitly
> > > assumes that all of its inputs lie in the range [0...2^32-1] and are
> integers.
> >
> > cvttsd2si truncates the result by rounding toward zero. So when
changing
the
> > range from [0...2^32-1] to [-2^31, 2^31-1], we need to compensate the
result.
>
> Well, i don't see your comment quite clearly.
>
> If the input lies in the range [0...2^32-1] and are integers, the check
is
> redundant I think.(So the test case should be re-written to only feed
integers.)
> But could you please share where does such explicit assumption come
from?
>
This is by design. All operators (both simplified and machine) that have
Change
in the name only change representations, and don't do truncations. In this
case,
it's not easy to tell that SSEFloat64ToInt32 should have assume this. But
you're
right, the test should only feed integers in the appropriate range.
> Thanks
> -Weiliang
Thanks for sharing. Please take a look at the second patch set.
Thanks
-Weiliang
https://codereview.chromium.org/582713002/
--
--
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.
