Eliminate unused function KeyedStoreIC::GenerateExtendStorage, and unused par...

Eliminate unused function KeyedStoreIC::GenerateExtendStorage, and unused parameter from StubCompiler::GenerateStoreField.

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

[William Hesse, 2010-02-05 13:54:22]

A small refactoring and fix for you.

[Kevin Millikin (Chromium), 2010-02-08 10:10:22]

I'm not sure the level of indirection is helping us here.

http://codereview.chromium.org/575022/diff/8001/9008
File src/arm/ic-arm.cc (right):

http://codereview.chromium.org/575022/diff/8001/9008#newcode800
src/arm/ic-arm.cc:800: void StoreIC::GenerateExtendStorage(MacroAssembler* masm)
{
Is there a reason to keep this function?

http://codereview.chromium.org/575022/diff/8001/9007
File src/arm/stub-cache-arm.cc (right):

http://codereview.chromium.org/575022/diff/8001/9007#newcode294
src/arm/stub-cache-arm.cc:294: if (!receiver_reg.is(r3)) {
__ push(receiver_reg);
__ mov(r2, Operand(Handle<Map>(transition)));
__ stm(db_w, sp, r2.bit() | r0.bit());
__ TailCallRuntime(
       ExternalReference(IC_Utility(kSharedStoreIC_ExtendStorage)),3, 1);

http://codereview.chromium.org/575022/diff/8001/9009
File src/builtins.h (right):

http://codereview.chromium.org/575022/diff/8001/9009#newcode72
src/builtins.h:72: V(StoreIC_ExtendStorage,      BUILTIN, UNINITIALIZED)        
          \
Is there a reason to keep this builtin?

http://codereview.chromium.org/575022/diff/8001/9001
File src/ia32/stub-cache-ia32.cc (right):

http://codereview.chromium.org/575022/diff/8001/9001#newcode640
src/ia32/stub-cache-ia32.cc:640: if (!receiver_reg.is(edx)) {
__ pop(scratch);  // Return address.
__ push(receiver_reg);
__ push(Immediate(Handle<Map>(transition)));
__ push(eax);
__ push(scratch);
__ TailCallRuntime(
       ExternalReference(IC_Utility(kSharedStoreIC_ExtendStorage)), 3, 1);

[William Hesse, 2010-02-09 09:17:52]

All suggested changes made.

http://codereview.chromium.org/575022/diff/8001/9008
File src/arm/ic-arm.cc (right):

http://codereview.chromium.org/575022/diff/8001/9008#newcode800
src/arm/ic-arm.cc:800: void StoreIC::GenerateExtendStorage(MacroAssembler* masm)
{
On 2010/02/08 10:10:22, Kevin Millikin wrote:
> Is there a reason to keep this function?

No.  It has been removed on all platforms.

http://codereview.chromium.org/575022/diff/8001/9007
File src/arm/stub-cache-arm.cc (right):

http://codereview.chromium.org/575022/diff/8001/9007#newcode294
src/arm/stub-cache-arm.cc:294: if (!receiver_reg.is(r3)) {
On 2010/02/08 10:10:22, Kevin Millikin wrote:
> __ push(receiver_reg);
> __ mov(r2, Operand(Handle<Map>(transition)));
> __ stm(db_w, sp, r2.bit() | r0.bit());
> __ TailCallRuntime(
>        ExternalReference(IC_Utility(kSharedStoreIC_ExtendStorage)),3, 1);

Done.

http://codereview.chromium.org/575022/diff/8001/9004
File src/builtins.cc (right):

http://codereview.chromium.org/575022/diff/8001/9004#newcode671
src/builtins.cc:671: static void Generate_StoreIC_ExtendStorage(MacroAssembler*
masm) {
This function removed.

http://codereview.chromium.org/575022/diff/8001/9009
File src/builtins.h (right):

http://codereview.chromium.org/575022/diff/8001/9009#newcode72
src/builtins.h:72: V(StoreIC_ExtendStorage,      BUILTIN, UNINITIALIZED)        
          \
On 2010/02/08 10:10:22, Kevin Millikin wrote:
> Is there a reason to keep this builtin?

No.  It has been removed.

http://codereview.chromium.org/575022/diff/8001/9001
File src/ia32/stub-cache-ia32.cc (right):

http://codereview.chromium.org/575022/diff/8001/9001#newcode640
src/ia32/stub-cache-ia32.cc:640: if (!receiver_reg.is(edx)) {
On 2010/02/08 10:10:22, Kevin Millikin wrote:
> __ pop(scratch);  // Return address.
> __ push(receiver_reg);
> __ push(Immediate(Handle<Map>(transition)));
> __ push(eax);
> __ push(scratch);
> __ TailCallRuntime(
>        ExternalReference(IC_Utility(kSharedStoreIC_ExtendStorage)), 3, 1);
> 

Done.

http://codereview.chromium.org/575022/diff/8001/9003
File src/ic.h (right):

http://codereview.chromium.org/575022/diff/8001/9003#newcode354
src/ic.h:354: static void GenerateExtendStorage(MacroAssembler* masm);
This function removed.

http://codereview.chromium.org/575022/diff/8001/9005
File src/x64/stub-cache-x64.cc (right):

http://codereview.chromium.org/575022/diff/8001/9005#newcode239
src/x64/stub-cache-x64.cc:239: Handle<Code>
ic(Builtins::builtin(Builtins::StoreIC_ExtendStorage));
Changed to tail-call runtime directly.

[Kevin Millikin (Chromium), 2010-02-09 15:09:52]

LGTM.

http://codereview.chromium.org/575022/diff/11001/12005
File src/x64/stub-cache-x64.cc (right):

http://codereview.chromium.org/575022/diff/11001/12005#newcode235
src/x64/stub-cache-x64.cc:235: __ pop(scratch);
Comment that you're popping the return address.  Simple "// Return address." is
enough.

[William Hesse, 2010-02-09 15:17:03]

http://codereview.chromium.org/575022/diff/11001/12005
File src/x64/stub-cache-x64.cc (right):

http://codereview.chromium.org/575022/diff/11001/12005#newcode235
src/x64/stub-cache-x64.cc:235: __ pop(scratch);
On 2010/02/09 15:09:52, Kevin Millikin wrote:
> Comment that you're popping the return address.  Simple "// Return address."
is
> enough.

Done.