MIPS: Refactoring after 'MIPS: split api call stubs into accessor and function call stubs'.

MIPS: Refactoring after 'MIPS: split api call stubs into accessor and function call stubs'.

Change stack handling to clarify the usage of reserved MIPS argument slots for mips32.

BUG=

Committed: https://crrev.com/0e7379e001a4d1adbc56da264316bfcddb8281a6
Cr-Commit-Position: refs/heads/master@{#26279}

[paul.l..., 2015-01-23 15:25:30]

paul.lind@imgtec.com changed reviewers:
+ balazs.kilvady@imgtec.com, dusan.milosavljevic@imgtec.com,
paul.lind@imgtec.com

[paul.l..., 2015-01-23 15:25:31]

https://codereview.chromium.org/867183003/diff/1/src/mips/code-stubs-mips.cc
File src/mips/code-stubs-mips.cc (right):

https://codereview.chromium.org/867183003/diff/1/src/mips/code-stubs-mips.cc#...
src/mips/code-stubs-mips.cc:4946: // ExitFrame contains four argument slots
after DirectCEntryStub call so
This should be "four MIPS argument slots", so that there is no confusion with JS
arguments, which are all over this function :)

https://codereview.chromium.org/867183003/diff/1/src/mips64/code-stubs-mips64.cc
File src/mips64/code-stubs-mips64.cc (right):

https://codereview.chromium.org/867183003/diff/1/src/mips64/code-stubs-mips64...
src/mips64/code-stubs-mips64.cc:4989: __ ld(s0, MemOperand(sp,
stack_space_offset + kCArgsSlotsSize));
I like leaving the DCHECK there to make sure it is 0. But since it is 0, you do
not have to add it to the MemOperand.  

Also see the TODO's in constants-mips64.h, I think kCArgsSlotsSize should
eventually be removed for mips64, since we are _only_ supporting N64. The way
this is done now with the ABI check was part of a long-ago plan for a single
port for 32 & 64, that we abandoned.

[balazs.kilvady, 2015-01-23 19:17:56]

I fixed the nits.

[paul.l..., 2015-01-23 22:05:16]

A comment nit, and another in the title: this technically is not a "fix", so
please remove the nested "Fix afters'. Maybe call this 'Cleanup after, or
refactor, or something. You might even add a second line to commit message with
something like "Change stack handling to clarify the usage of reserved MIPS
argument slots for mips32."

And please add some google team reviewers including Dan C, and cc v8-dev.

https://codereview.chromium.org/867183003/diff/20001/src/mips/code-stubs-mips.cc
File src/mips/code-stubs-mips.cc (right):

https://codereview.chromium.org/867183003/diff/20001/src/mips/code-stubs-mips...
src/mips/code-stubs-mips.cc:4947: // so this must be acounted for.
nit: spelling: 'accounted'

[balazs.kilvady, 2015-01-26 11:07:17]

https://codereview.chromium.org/867183003/diff/1/src/mips/code-stubs-mips.cc
File src/mips/code-stubs-mips.cc (right):

https://codereview.chromium.org/867183003/diff/1/src/mips/code-stubs-mips.cc#...
src/mips/code-stubs-mips.cc:4946: // ExitFrame contains four argument slots
after DirectCEntryStub call so
On 2015/01/23 15:25:30, paul.l... wrote:
> This should be "four MIPS argument slots", so that there is no confusion with
JS
> arguments, which are all over this function :)

Done.

https://codereview.chromium.org/867183003/diff/1/src/mips64/code-stubs-mips64.cc
File src/mips64/code-stubs-mips64.cc (right):

https://codereview.chromium.org/867183003/diff/1/src/mips64/code-stubs-mips64...
src/mips64/code-stubs-mips64.cc:4989: __ ld(s0, MemOperand(sp,
stack_space_offset + kCArgsSlotsSize));
On 2015/01/23 15:25:30, paul.l... wrote:
> I like leaving the DCHECK there to make sure it is 0. But since it is 0, you
do
> not have to add it to the MemOperand.  
> 
> Also see the TODO's in constants-mips64.h, I think kCArgsSlotsSize should
> eventually be removed for mips64, since we are _only_ supporting N64. The way
> this is done now with the ABI check was part of a long-ago plan for a single
> port for 32 & 64, that we abandoned.

Done.

https://codereview.chromium.org/867183003/diff/20001/src/mips/code-stubs-mips.cc
File src/mips/code-stubs-mips.cc (right):

https://codereview.chromium.org/867183003/diff/20001/src/mips/code-stubs-mips...
src/mips/code-stubs-mips.cc:4947: // so this must be acounted for.
On 2015/01/23 22:05:16, paul.l... wrote:
> nit: spelling: 'accounted'

Done.

[paul.l..., 2015-01-26 16:27:39]

paul.lind@imgtec.com changed reviewers:
+ danno@chromium.org, dcarney@chromium.org

[paul.l..., 2015-01-26 16:27:39]

LGTM.

[balazs.kilvady, 2015-01-26 16:29:12]

The CQ bit was checked by balazs.kilvady@imgtec.com

[commit-bot: I haz the power, 2015-01-26 16:29:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/867183003/40001

[commit-bot: I haz the power, 2015-01-26 17:02:04]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-01-26 17:02:17]

Patchset 3 (id:??) landed as
https://crrev.com/0e7379e001a4d1adbc56da264316bfcddb8281a6
Cr-Commit-Position: refs/heads/master@{#26279}