Inline in-object property stores when in loop and not in top-level

Inline in-object property stores on ia32 when in loop and not in top-level code.

Committed: http://code.google.com/p/v8/source/detail?r=5105

[Vyacheslav Egorov (Chromium), 2010-07-20 14:22:12]

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.

[Mads Ager (chromium), 2010-07-21 06:55:01]

Thanks! Added bug report for porting to other architectures and updated TODOs.

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();
On 2010/07/20 14:22:12, Vyacheslav Egorov wrote:
> comment why we require nop here would be great. 

Done.

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;
On 2010/07/20 14:22:12, Vyacheslav Egorov wrote:
> not having - kHeapObjectTag here is pretty confusing from my point of view. 

You are right. Added.

http://codereview.chromium.org/3046006/diff/8001/9003#newcode1828
src/ia32/ic-ia32.cc:1828: const int StoreIC::kOffsetToStoreInstruction = 13;
On 2010/07/20 14:22:12, Vyacheslav Egorov wrote:
> It might be reasonable to add some ASSERTs to PatchInlinedStore to actually
> check that assembly looks as expected.

I added asserts to PatchInlinedStore that check that the offsets are as expected
(either initial value, reset value or we are resetting). That would be very
unlikely to be the case if the instruction stream is not generated for an
inlined store.