LGTM with comments addressed
http://codereview.chromium.org/6794052/diff/1/src/ia32/code-stubs-ia32.cc File src/ia32/code-stubs-ia32.cc (right): http://codereview.chromium.org/6794052/diff/1/src/ia32/code-stubs-ia32.cc#newcode6476 src/ia32/code-stubs-ia32.cc:6476: // the object we are writing into in the 'address' register. We insert a we have the object in the object register. I guess we have something different in the address register. Like slot offset. http://codereview.chromium.org/6794052/diff/1/src/ia32/codegen-ia32.cc File src/ia32/codegen-ia32.cc (right): http://codereview.chromium.org/6794052/diff/1/src/ia32/codegen-ia32.cc#newcode9798 src/ia32/codegen-ia32.cc:9798: __ RememberedSetHelper(receiver.reg(), this place should have an incremental write barrier. why did you remove it? http://codereview.chromium.org/6794052/diff/1/src/ia32/lithium-codegen-ia32.cc File src/ia32/lithium-codegen-ia32.cc (right): http://codereview.chromium.org/6794052/diff/1/src/ia32/lithium-codegen-ia32.cc#newcode2056 src/ia32/lithium-codegen-ia32.cc:2056: __ RecordWrite(object, address, value, OMIT_REMEMBERED_SET, kSaveFPRegs); mixing two constant naming styles in a single call. maybe we should choose one? http://codereview.chromium.org/6794052/diff/1/src/ia32/macro-assembler-ia32.cc File src/ia32/macro-assembler-ia32.cc (right): http://codereview.chromium.org/6794052/diff/1/src/ia32/macro-assembler-ia32.cc#newcode67 src/ia32/macro-assembler-ia32.cc:67: preserve[object.code()] = true; Is there any particular reason for modes removal? This will probably make call prologue bigger in some cases. http://codereview.chromium.org/6794052/diff/1/src/ia32/macro-assembler-ia32.cc#newcode176 src/ia32/macro-assembler-ia32.cc:176: if (emit_remembered_set == OMIT_REMEMBERED_SET && Maybe add an assertion that during snapshot generation we don't omit any write barriers. http://codereview.chromium.org/6794052/diff/1/src/ia32/stub-cache-ia32.cc File src/ia32/stub-cache-ia32.cc (right): http://codereview.chromium.org/6794052/diff/1/src/ia32/stub-cache-ia32.cc#newcode1502 src/ia32/stub-cache-ia32.cc:1502: EMIT_REMEMBERED_SET, kDontSaveFPRegs, OMIT_SMI_CHECK); strange formatting http://codereview.chromium.org/6794052/diff/1/src/ia32/stub-cache-ia32.cc#newcode1545 src/ia32/stub-cache-ia32.cc:1545: // write too. this was not a record write for hole, it's record write for a value we push on line 1535 http://codereview.chromium.org/6794052/diff/1/src/incremental-marking.cc File src/incremental-marking.cc (right): http://codereview.chromium.org/6794052/diff/1/src/incremental-marking.cc#newcode172 src/incremental-marking.cc:172: CodeStub::IncrementalMarkingRecordWrite) { maybe change keyname as well? http://codereview.chromium.org/6794052/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
