Inline %_ArgumentsLength.

Inline %_ArgumentsLength.

This makes usage of cmov unnecessary.

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

[Vitaly Repeshko, 2010-03-30 13:17:23]

Florian,

Can you please have a look too?


Thanks,
Vitaly

[antonm, 2010-03-30 13:28:05]

LGTM

http://codereview.chromium.org/1533004/diff/1/5
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/1533004/diff/1/5#newcode6233
src/ia32/codegen-ia32.cc:6233: Result result = allocator_->Allocate();
shouldn't you Unuse fp and result at the end of the function?  it looks like a
common convention.

http://codereview.chromium.org/1533004/diff/1/5#newcode6233
src/ia32/codegen-ia32.cc:6233: Result result = allocator_->Allocate();
just curious: any chances allocating the single reg would be faster even given
that control flow would become more involved?

http://codereview.chromium.org/1533004/diff/1/5#newcode6254
src/ia32/codegen-ia32.cc:6254: if (FLAG_debug_code) __
AbortIfNotSmi(result.reg());
two questions.

1) why not use cmov still?
2) do we know what happens more often?  there is a redundant load to
result.reg(), of course, that's immediate so it hardly matters, just curious.

[Vitaly Repeshko, 2010-03-30 13:40:30]

Anton, thanks for your comments!


-- Vitaly

http://codereview.chromium.org/1533004/diff/1/5
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/1533004/diff/1/5#newcode6233
src/ia32/codegen-ia32.cc:6233: Result result = allocator_->Allocate();
On 2010/03/30 13:28:05, antonm wrote:
> shouldn't you Unuse fp and result at the end of the function?  it looks like a
> common convention.

~Result will do it. I think it's only necessary when having some complicated
control flow that actually checks there are no off-frame values.

http://codereview.chromium.org/1533004/diff/1/5#newcode6233
src/ia32/codegen-ia32.cc:6233: Result result = allocator_->Allocate();
On 2010/03/30 13:28:05, antonm wrote:
> just curious: any chances allocating the single reg would be faster even given
> that control flow would become more involved?

Hard to tell. I can experiment more after this change is submitted.

http://codereview.chromium.org/1533004/diff/1/5#newcode6254
src/ia32/codegen-ia32.cc:6254: if (FLAG_debug_code) __
AbortIfNotSmi(result.reg());
On 2010/03/30 13:28:05, antonm wrote:
> two questions.
> 
> 1) why not use cmov still?

AFAIK, cmov should be used in totally unpredictable cases and it made perfect
sense for the shared stub. Now that it's inlined I think the branch predictor
should be able to do a better job.

> 2) do we know what happens more often?  there is a redundant load to
> result.reg(), of course, that's immediate so it hardly matters, just curious.

I think we get the expected number of arguments (i.e. equal to the number of
formal parameters) more often.

[fschneider, 2010-03-30 13:44:36]

LGTM.

I think this is a border-line case for cmov. I'm fine not using cmov here: It
probably doesn't make a big difference. cmov is a little more compact, but since
we don't inline this in a lot of places (only our built-ins) the code-size
reduction is not big either.

http://codereview.chromium.org/1533004/diff/1/5
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/1533004/diff/1/5#newcode6233
src/ia32/codegen-ia32.cc:6233: Result result = allocator_->Allocate();
On 2010/03/30 13:28:05, antonm wrote:
> shouldn't you Unuse fp and result at the end of the function?  it looks like a
> common convention.

Unuse() is called by the destructor of Result at the end. In this case since we
don't allocate other things before that there is no need to explicitly call
Unuse.

[Vitaly Repeshko, 2010-03-30 13:55:35]

Thanks, guys, for speedy review! Submitted.


-- Vitaly