Use the HandlerFrontendHeader in most call compilers.

Use the HandlerFrontendHeader in most call compilers.

R=ishell@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=18125

[Toon Verwaest, 2013-11-22 17:04:15]

PTAL

[Igor Sheludko, 2013-11-25 16:31:53]

I commented arm sources but the same notes are related to other platforms'
sources.

lgtm with notes.

https://codereview.chromium.org/83753002/diff/70001/src/arm/stub-cache-arm.cc
File src/arm/stub-cache-arm.cc (left):

https://codereview.chromium.org/83753002/diff/70001/src/arm/stub-cache-arm.cc...
src/arm/stub-cache-arm.cc:766: 
Non-equivalent modification for one of the GenerateCallFunction() usages. See
below.

https://codereview.chromium.org/83753002/diff/70001/src/arm/stub-cache-arm.cc...
src/arm/stub-cache-arm.cc:1617: // -----------------------------------
Are these (and similar) comments really useless here?

https://codereview.chromium.org/83753002/diff/70001/src/arm/stub-cache-arm.cc
File src/arm/stub-cache-arm.cc (right):

https://codereview.chromium.org/83753002/diff/70001/src/arm/stub-cache-arm.cc...
src/arm/stub-cache-arm.cc:1613: HandlerFrontendHeader(object, holder, name,
RECEIVER_MAP_CHECK, &miss);
Eagle's eye!

https://codereview.chromium.org/83753002/diff/70001/src/arm/stub-cache-arm.cc...
src/arm/stub-cache-arm.cc:2380: // -----------------------------------
What do you think about putting these comments here:
   //  -- sp[(argc - n - 1) * 4] : arg[n] (zero-based)	
   //  -- ...	
   //  -- sp[argc * 4]           : receiver	
   // -----------------------------------

https://codereview.chromium.org/83753002/diff/70001/src/arm/stub-cache-arm.cc...
src/arm/stub-cache-arm.cc:2383: Register reg = r1;
It looks like the other usages of "r1" below should be replaced with "reg".

https://codereview.chromium.org/83753002/diff/70001/src/arm/stub-cache-arm.cc...
src/arm/stub-cache-arm.cc:2524: GenerateCallFunction(masm(), object,
arguments(), &miss, extra_state_);
Modified GenerateCallFunction() is no longer patches the receiver on the stack.
Is it true that we no longer need this check here?

https://codereview.chromium.org/83753002/diff/70001/src/stub-cache.h
File src/stub-cache.h (right):

https://codereview.chromium.org/83753002/diff/70001/src/stub-cache.h#newcode909
src/stub-cache.h:909: Register HandlerFrontendHeader(Handle<Object> object,
Probably is it worth adding a comment about what is returned from the function.

[Toon Verwaest, 2013-11-27 15:16:55]

Please take another look. That bug you noticed took quite some effort to clean
up... Now I always move the receiver into r0 on arm, and the method will end up
in r1. This means that in some cases I'll have to keep the receiver in r0 and
use r1 for the result until the end where I move r1 back over r0.

https://chromiumcodereview.appspot.com/83753002/diff/70001/src/arm/stub-cache...
File src/arm/stub-cache-arm.cc (left):

https://chromiumcodereview.appspot.com/83753002/diff/70001/src/arm/stub-cache...
src/arm/stub-cache-arm.cc:766: 
On 2013/11/25 16:31:53, Igor Sheludko wrote:
> Non-equivalent modification for one of the GenerateCallFunction() usages. See
> below.

Done.

https://chromiumcodereview.appspot.com/83753002/diff/70001/src/arm/stub-cache...
src/arm/stub-cache-arm.cc:1617: // -----------------------------------
Yes they are. Look at Load/Store, that's what I want to achieve. They are
platform independent, and all configurations of variables aren't done anymore in
comments but by using abstractions for the variables.

On 2013/11/25 16:31:53, Igor Sheludko wrote:
> Are these (and similar) comments really useless here?

https://chromiumcodereview.appspot.com/83753002/diff/70001/src/arm/stub-cache...
File src/arm/stub-cache-arm.cc (right):

https://chromiumcodereview.appspot.com/83753002/diff/70001/src/arm/stub-cache...
src/arm/stub-cache-arm.cc:2380: // -----------------------------------
I removed the comment altogether. I prefer to add more abstractions over time
(eg, LoadReceiver(reg)) than adding comments.

On 2013/11/25 16:31:53, Igor Sheludko wrote:
> What do you think about putting these comments here:
>    //  -- sp[(argc - n - 1) * 4] : arg[n] (zero-based)	
>    //  -- ...	
>    //  -- sp[argc * 4]           : receiver	
>    // -----------------------------------

https://chromiumcodereview.appspot.com/83753002/diff/70001/src/arm/stub-cache...
src/arm/stub-cache-arm.cc:2383: Register reg = r1;
On 2013/11/25 16:31:53, Igor Sheludko wrote:
> It looks like the other usages of "r1" below should be replaced with "reg".

Done.

https://chromiumcodereview.appspot.com/83753002/diff/70001/src/arm/stub-cache...
src/arm/stub-cache-arm.cc:2524: GenerateCallFunction(masm(), object,
arguments(), &miss, extra_state_);
On 2013/11/25 16:31:53, Igor Sheludko wrote:
> Modified GenerateCallFunction() is no longer patches the receiver on the
stack.
> Is it true that we no longer need this check here?

Done.

https://chromiumcodereview.appspot.com/83753002/diff/70001/src/stub-cache.h
File src/stub-cache.h (right):

https://chromiumcodereview.appspot.com/83753002/diff/70001/src/stub-cache.h#n...
src/stub-cache.h:909: Register HandlerFrontendHeader(Handle<Object> object,
On 2013/11/25 16:31:53, Igor Sheludko wrote:
> Probably is it worth adding a comment about what is returned from the
function.

Done.

[Igor Sheludko, 2013-11-28 12:54:27]

lgtm with nits which as I saw you already address in the next CL.

https://codereview.chromium.org/83753002/diff/110001/src/arm/stub-cache-arm.cc
File src/arm/stub-cache-arm.cc (right):

https://codereview.chromium.org/83753002/diff/110001/src/arm/stub-cache-arm.c...
src/arm/stub-cache-arm.cc:760: if (object->IsGlobalObject()) {
Looks like PatchGlobalProxy(object);
Why don't make GenerateCallFunction() private method and call the patch
function?

https://codereview.chromium.org/83753002/diff/110001/src/ia32/stub-cache-ia32.cc
File src/ia32/stub-cache-ia32.cc (right):

https://codereview.chromium.org/83753002/diff/110001/src/ia32/stub-cache-ia32...
src/ia32/stub-cache-ia32.cc:2656: if (object->IsGlobalObject()) {
Looks like PatchGlobalProxy(object);

https://codereview.chromium.org/83753002/diff/110001/src/x64/stub-cache-x64.cc
File src/x64/stub-cache-x64.cc (right):

https://codereview.chromium.org/83753002/diff/110001/src/x64/stub-cache-x64.c...
src/x64/stub-cache-x64.cc:2577: if (object->IsGlobalObject()) {
Looks like PatchGlobalProxy(object);

[Toon Verwaest, 2013-11-28 13:56:55]

Committed patchset #4 manually as r18125 (presubmit successful).