Change LoadIC interface on ia32 to take arguments in registers.

Change LoadIC interface on ia32 to take arguments in registers.

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

[Kevin Millikin (Chromium), 2010-02-05 09:02:33]

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'

[William Hesse, 2010-02-12 10:21:39]

All changes made, except the cleanup of EmitNamedPropertyLoad in the full code
generator.

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);
On 2010/02/05 09:02:33, Kevin Millikin wrote:
> This needs a comment that you are copying the temporary (the object) with one
> copy allocated to the accumulator and one to the stack.

Done.

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

Will do later.

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_) {
On 2010/02/05 09:02:33, Kevin Millikin wrote:
> 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.

Done.

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);
On 2010/02/05 09:02:33, Kevin Millikin wrote:
> Add a simple comment that you are copying the value, one copy in the
accumulator
> and one on the stack.

Done.

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) {
On 2010/02/05 09:02:33, Kevin Millikin wrote:
> As with the store ICs, I think you can rename Generate ==> GenerateMiss and
> remove the extra argument.

Done.

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.
On 2010/02/05 09:02:33, Kevin Millikin wrote:
> containing ==> containin'

Done.