LGTM

http://codereview.chromium.org/3046006/diff/8001/9002
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/3046006/diff/8001/9002#newcode5343
src/ia32/codegen-ia32.cc:5343: __ nop();
comment why we require nop here would be great.

http://codereview.chromium.org/3046006/diff/8001/9003
File src/ia32/ic-ia32.cc (right):

http://codereview.chromium.org/3046006/diff/8001/9003#newcode1708
src/ia32/ic-ia32.cc:1708: *reinterpret_cast<int*>(offset_address) =
offset;
not having - kHeapObjectTag here is pretty confusing from my point of
view.

http://codereview.chromium.org/3046006/diff/8001/9003#newcode1828
src/ia32/ic-ia32.cc:1828: const int StoreIC::kOffsetToStoreInstruction =
13;
It might be reasonable to add some ASSERTs to PatchInlinedStore to
actually check that assembly looks as expected.

http://codereview.chromium.org/3046006/show

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

Reply via email to