On 2014/09/18 16:04:11, Weiliang wrote:
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
lgtm
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.
