LGTM.

http://codereview.chromium.org/573009/diff/15/26
File src/arm/stub-cache-arm.cc (right):

http://codereview.chromium.org/573009/diff/15/26#newcode219
src/arm/stub-cache-arm.cc:219: Register receiver,
Fix indentation here.

http://codereview.chromium.org/573009/diff/15/29
File src/full-codegen.cc (right):

http://codereview.chromium.org/573009/diff/15/29#newcode1055
src/full-codegen.cc:1055: VisitForValue(prop->obj(), kAccumulator);
This needs a comment that you are copying the temporary (the object)
with one copy allocated to the accumulator and one to the stack.

http://codereview.chromium.org/573009/diff/15/29#newcode1078
src/full-codegen.cc:1078: EmitNamedPropertyLoad(prop);
As a simple cleanup in another change, could you make
EmitNamedPropertyLoad/EmitKeyedPropertyLoad use the expression context
to determine what to do with their result?

http://codereview.chromium.org/573009/diff/15/18
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/573009/diff/15/18#newcode6593
src/ia32/codegen-ia32.cc:6593: if (persist_after_get_) {
Since this code and the corresponding !persist_after_get_ bracket the
load in both arms of the if, I think it's more obvious to have it
outside the if.

http://codereview.chromium.org/573009/diff/15/17
File src/ia32/full-codegen-ia32.cc (right):

http://codereview.chromium.org/573009/diff/15/17#newcode1567
src/ia32/full-codegen-ia32.cc:1567: VisitForValue(prop->obj(),
kAccumulator);
Add a simple comment that you are copying the value, one copy in the
accumulator and one on the stack.

http://codereview.chromium.org/573009/diff/15/23
File src/ia32/ic-ia32.cc (right):

http://codereview.chromium.org/573009/diff/15/23#newcode1231
src/ia32/ic-ia32.cc:1231: void LoadIC::Generate(MacroAssembler* masm,
const ExternalReference& f) {
As with the store ICs, I think you can rename Generate ==> GenerateMiss
and remove the extra argument.

http://codereview.chromium.org/573009/diff/15/19
File src/ia32/stub-cache-ia32.cc (right):

http://codereview.chromium.org/573009/diff/15/19#newcode721
src/ia32/stub-cache-ia32.cc:721: // Return the register containing the
holder.
containing ==> containin'

http://codereview.chromium.org/573009

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

Reply via email to