On 2013/09/18 10:28:14, Bangfu wrote:
On 2013/09/18 09:26:18, Benedikt Meurer wrote:
> On 2013/09/18 07:20:46, Bangfu wrote:
> > On 2013/09/13 10:16:58, Bangfu wrote:
> > > On 2013/09/13 09:15:23, oliv wrote:
> > > > On 2013/09/12 07:32:48, Bangfu wrote:
> > > > > On 2013/09/12 07:23:40, Benedikt Meurer wrote:
> > > > > > LGTM with nits.
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
https://codereview.chromium.org/23496041/diff/4002/src/arm/lithium-codegen-arm.cc
> > > > > > File src/arm/lithium-codegen-arm.cc (right):
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
https://codereview.chromium.org/23496041/diff/4002/src/arm/lithium-codegen-arm.cc#newcode4894
> > > > > > src/arm/lithium-codegen-arm.cc:4894: __ b(eq, &heap_number);
> > > > > > Nit: indentation.
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
https://codereview.chromium.org/23496041/diff/4002/src/arm/lithium-codegen-arm.cc#newcode4937
> > > > > > src/arm/lithium-codegen-arm.cc:4937:
> > > > > > Nit: new line.
> > > > >
> > > > > Nit: Updated.
> > > >
> > > > hei, I really like the idea of this patch, since e.g. on x64 i
verified
> that
> > > we
> > > > spend 10% of the untagging cpu cycles on this branch! but it took
me a
> while
> > > to
> > > > understand what your code was doing. maybe you could have a look
at my
> port:
> > > > https://codereview.chromium.org/23872026/.
> > > >
> > > > cheers
> > >
> > > Thanks for review.
> >
> > Do you want me to close this issue?
>
> Sorry for the delay... No, can you please simplify your patch similar to
what
> olivf@ did with the Intel patch. The most important point IMHO is to
avoid
the
> conditional branch on the common execution path.
I've looked x64 version code, in common execution path it has:
1. smi check
2. heap number check
3. load value
4. conditional branch to convert
5. unconditional branch to done
In our ARM patch, in common execution path it has:
1. smi check
2. heap number check
3. conditional load value
4. conditional branch to done
Right. It'd be nice to move the conditional load out of the if's and
replace the
conditional branch to done with a conditional branch to convert (not taken
in
the common case) and the unconditional branch to done.
https://codereview.chromium.org/23496041/
--
--
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/groups/opt_out.
