LGTM.

http://codereview.chromium.org/553149/diff/2001/2004
File src/arm/fast-codegen-arm.cc (right):

http://codereview.chromium.org/553149/diff/2001/2004#newcode72
src/arm/fast-codegen-arm.cc:72: int index = 2 +
function()->scope()->num_parameters();
Maybe refactor the code that loads this from the stack? It's used in two
places.

http://codereview.chromium.org/553149/diff/2001/2005
File src/arm/full-codegen-arm.cc (right):

http://codereview.chromium.org/553149/diff/2001/2005#newcode84
src/arm/full-codegen-arm.cc:84: __ CallRuntime(Runtime::kNewContext, 1);
Consider using the faster new context stub if possible. At least add a
TODO.

http://codereview.chromium.org/553149/diff/2001/2005#newcode99
src/arm/full-codegen-arm.cc:99: __ str(r0, MemOperand(cp,
Context::SlotOffset(slot->index())));
This doesn't use the write barrier and that is safe because? At least we
should have a comment.

http://codereview.chromium.org/553149/diff/2001/2005#newcode116
src/arm/full-codegen-arm.cc:116: fun->num_parameters() * kPointerSize));
Weird indentation.

http://codereview.chromium.org/553149/diff/2001/2007
File src/compiler.h (right):

http://codereview.chromium.org/553149/diff/2001/2007#newcode61
src/compiler.h:61: bool has_globals() { return has_globals_; }
has_this_properties, has_globals could be renamed to
accesses_this_properties and accesses_globals. I don't know if that's
better though.

http://codereview.chromium.org/553149/diff/2001/2013
File src/full-codegen.h (right):

http://codereview.chromium.org/553149/diff/2001/2013#newcode66
src/full-codegen.h:66: enum Mode {
So you're falling back to the full code generator (SECONDARY), not to
the old-school optimizing one?

http://codereview.chromium.org/553149/diff/2001/2015
File src/ia32/fast-codegen-ia32.cc (right):

http://codereview.chromium.org/553149/diff/2001/2015#newcode44
src/ia32/fast-codegen-ia32.cc:44: // Offset 2 is due to return address
and saved frame pointer.
Refactor loading of this.

http://codereview.chromium.org/553149/diff/2001/2015#newcode67
src/ia32/fast-codegen-ia32.cc:67: // Restore this.
Refactor loading of this.

http://codereview.chromium.org/553149/diff/2001/2016
File src/ia32/full-codegen-ia32.cc (right):

http://codereview.chromium.org/553149/diff/2001/2016#newcode83
src/ia32/full-codegen-ia32.cc:83: __ CallRuntime(Runtime::kNewContext,
1);
Use fast new context stub.

http://codereview.chromium.org/553149/diff/2001/2016#newcode99
src/ia32/full-codegen-ia32.cc:99: __ mov(Operand(esi,
Context::SlotOffset(slot->index())), eax);
Comment on missing write barrier?

http://codereview.chromium.org/553149/diff/2001/2018
File src/x64/fast-codegen-x64.cc (right):

http://codereview.chromium.org/553149/diff/2001/2018#newcode46
src/x64/fast-codegen-x64.cc:46: __ movq(rdx, Operand(rbp, index *
kPointerSize));
Refactor loading of this.

http://codereview.chromium.org/553149/diff/2001/2018#newcode68
src/x64/fast-codegen-x64.cc:68: // Offset 2 is due to return address and
saved frame pointer.
Refactor loading of this.

http://codereview.chromium.org/553149/diff/2001/2019
File src/x64/full-codegen-x64.cc (right):

http://codereview.chromium.org/553149/diff/2001/2019#newcode83
src/x64/full-codegen-x64.cc:83: __ CallRuntime(Runtime::kNewContext, 1);
Use fast new context stub.

http://codereview.chromium.org/553149/diff/2001/2019#newcode99
src/x64/full-codegen-x64.cc:99: __ movq(Operand(rsi,
Context::SlotOffset(slot->index())), rax);
Comment on missing write barrier.

http://codereview.chromium.org/553149

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to