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!

https://codereview.chromium.org/207543003/

--
--
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.

Reply via email to