On 2014/03/21 15:17:57, dcarney wrote:
On 2014/03/21 12:27:34, Yang wrote:
> On 2014/03/21 12:26:54, Yang wrote:
> > On 2014/03/21 09:52:22, Yang wrote:
> > > LGTM.
> > >
> > >
>
https://codereview.chromium.org/207543003/diff/1/src/ia32/full-codegen-ia32.cc
> > > File src/ia32/full-codegen-ia32.cc (right):
> > >
> > >
> >
>
https://codereview.chromium.org/207543003/diff/1/src/ia32/full-codegen-ia32.cc#newcode191
> > > src/ia32/full-codegen-ia32.cc:191:
> > > PrepareForBailoutForId(BailoutId::FunctionEntry(), NO_REGISTERS);
> > > On 2014/03/21 09:43:28, dcarney wrote:
> > > > no idea what this does, whether it's the right bailout id, or
whether
i
> can
> > > call
> > > > it twice
> > >
> > > I don't think we need this. I don't expect optimized code to somehow
> > deoptimize
> > > and end up here. (At least I think that's what this is for).
> > >
> > >
> >
>
https://codereview.chromium.org/207543003/diff/1/src/ia32/full-codegen-ia32.cc#newcode204
> > > src/ia32/full-codegen-ia32.cc:204: __ cmp(ecx,
Immediate(Smi::FromInt(0)));
> > > No need for this cmp. dec also sets the flags. Just do a j(not_zero,
> > > &loop_header) later. (not_zero is actually the same flag as
not_equal).
> >
> > LGTM, but...
> >
> > I kinda feel bad about this, but at the time we do the first stack
check,
we
> can
> > already know whether there is going to be a stack overflow, simply by
> comparing
> > (esp - locals_count) with the stack limit. We don't have to do it in
every
> loop.
> >
> > Having the loop there is a good idea nevertheless!
>
> (I was feeling bad about this for not noticing earlier and having you
port
to
> all platforms already).
new version with check before loop
LGTM.
https://codereview.chromium.org/207543003/
--
--
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.
