[turbofan]: Implement tail calls with more callee than caller parameters

[turbofan]: Implement tail calls with more callee than caller parameters

* Adds a PrepareForTailCall instruction that bumps the stack in the case that
  the number of parameters passed to the callee causes the stack to exceed the
  calleer's frame size.
* Uses the gap resolver to move the saved caller return address and frame
  pointer to the approprate location in the tail-called frame.

BUG=v8:4076
LOG=n

Committed: https://crrev.com/c6d310da4d86ce2d8178a49f4d34b66ad6143432
Cr-Commit-Position: refs/heads/master@{#32151}

[danno, 2015-11-19 09:45:18]

Description was changed from

==========
[turbofan]: Make tail calls with more callee than caller parameters work

* Adds a PrepareForTailCall instruction that bumps the stack in the case that
  the number of parameters passed to the callee causes the stack to exceed the
  calleer's frame size.
* Uses the gap resolver to move the saved caller return address and frame
  pointer to the approprate location in the tail-called frame.

BUG=v8:4076
LOG=n
==========

to

==========
[turbofan]: Implement tail calls with more callee than caller parameters

* Adds a PrepareForTailCall instruction that bumps the stack in the case that
  the number of parameters passed to the callee causes the stack to exceed the
  calleer's frame size.
* Uses the gap resolver to move the saved caller return address and frame
  pointer to the approprate location in the tail-called frame.

BUG=v8:4076
LOG=n
==========

[danno, 2015-11-19 09:58:56]

danno@chromium.org changed reviewers:
+ bmeurer@chromium.org, jarin@chromium.org

[danno, 2015-11-19 09:58:57]

PTAL

[danno, 2015-11-20 10:06:12]

danno@chromium.org changed reviewers:
+ mtrofin@chromium.org

[danno, 2015-11-20 10:06:12]

danno@chromium.org changed required reviewers:
+ bmeurer@chromium.org, jarin@chromium.org

[Benedikt Meurer, 2015-11-20 10:06:43]

LGTM with two beefs.

https://codereview.chromium.org/1455833004/diff/80002/src/compiler/instructio...
File src/compiler/instruction-selector-impl.h (right):

https://codereview.chromium.org/1455833004/diff/80002/src/compiler/instructio...
src/compiler/instruction-selector-impl.h:157:
UnallocatedOperand::cast(TempLocation(from_location, kMachPtr));
Is kMachPtr really what you want here? Shouldn't it be type too?

https://codereview.chromium.org/1455833004/diff/80002/src/compiler/linkage.h
File src/compiler/linkage.h (right):

https://codereview.chromium.org/1455833004/diff/80002/src/compiler/linkage.h#...
src/compiler/linkage.h:73: DCHECK(FLAG_enable_embedded_constant_pool);
This use of FLAG_something worries me. Can we somehow make that into a global
constant, because disabling that at runtime will not work anyway.

[Jarin, 2015-11-20 10:56:10]

lgtm with questions.

https://codereview.chromium.org/1455833004/diff/140001/src/compiler/arm/code-...
File src/compiler/arm/code-generator-arm.cc (right):

https://codereview.chromium.org/1455833004/diff/140001/src/compiler/arm/code-...
src/compiler/arm/code-generator-arm.cc:362: if (stack_param_delta != 0) {
Is the special casing for the stack_parameter_delta == 0 necessary?

https://codereview.chromium.org/1455833004/diff/140001/src/compiler/arm/code-...
src/compiler/arm/code-generator-arm.cc:391: total_discarded_slots -=
stack_param_delta;
The calculation of total_discarded_slots is repeated in the function above and
in (all?) other platforms. Could you extract it to a platform-independent
function?

https://codereview.chromium.org/1455833004/diff/140001/src/compiler/instructi...
File src/compiler/instruction-selector-impl.h (right):

https://codereview.chromium.org/1455833004/diff/140001/src/compiler/instructi...
src/compiler/instruction-selector-impl.h:158: from_location,
static_cast<MachineType>(kTypeAny | kMachPtr)));
Why does not this use the incoming type? As suggested in
instruction-selector.cc, we should perhaps rename the function to
UsePointerLocation and hardcode kMachPtr here.

https://codereview.chromium.org/1455833004/diff/140001/src/compiler/instructi...
File src/compiler/instruction-selector.cc (right):

https://codereview.chromium.org/1455833004/diff/140001/src/compiler/instructi...
src/compiler/instruction-selector.cc:521: saved_return_location, kMachPtr);
It looks like the UseLocation method is only called with kMachPtr. Perhaps you
could hardcode kMachPtr there?

[danno, 2015-11-20 14:43:24]

Feedback addressed and new version uploaded. I shall land it if the commit queue
bots so will it.

https://codereview.chromium.org/1455833004/diff/80002/src/compiler/instructio...
File src/compiler/instruction-selector-impl.h (right):

https://codereview.chromium.org/1455833004/diff/80002/src/compiler/instructio...
src/compiler/instruction-selector-impl.h:157:
UnallocatedOperand::cast(TempLocation(from_location, kMachPtr));
On 2015/11/20 10:06:43, Benedikt Meurer wrote:
> Is kMachPtr really what you want here? Shouldn't it be type too?

Done.

https://codereview.chromium.org/1455833004/diff/80002/src/compiler/linkage.h
File src/compiler/linkage.h (right):

https://codereview.chromium.org/1455833004/diff/80002/src/compiler/linkage.h#...
src/compiler/linkage.h:73: DCHECK(FLAG_enable_embedded_constant_pool);
On 2015/11/20 10:06:43, Benedikt Meurer wrote:
> This use of FLAG_something worries me. Can we somehow make that into a global
> constant, because disabling that at runtime will not work anyway.

Done.

https://codereview.chromium.org/1455833004/diff/140001/src/compiler/arm/code-...
File src/compiler/arm/code-generator-arm.cc (right):

https://codereview.chromium.org/1455833004/diff/140001/src/compiler/arm/code-...
src/compiler/arm/code-generator-arm.cc:362: if (stack_param_delta != 0) {
On 2015/11/20 10:56:09, Jarin wrote:
> Is the special casing for the stack_parameter_delta == 0 necessary?

No, but the code is more compact. For simplicity I'll remove the special case.

https://codereview.chromium.org/1455833004/diff/140001/src/compiler/arm/code-...
src/compiler/arm/code-generator-arm.cc:391: total_discarded_slots -=
stack_param_delta;
On 2015/11/20 10:56:09, Jarin wrote:
> The calculation of total_discarded_slots is repeated in the function above and
> in (all?) other platforms. Could you extract it to a platform-independent
> function?

Done.

https://codereview.chromium.org/1455833004/diff/140001/src/compiler/instructi...
File src/compiler/instruction-selector-impl.h (right):

https://codereview.chromium.org/1455833004/diff/140001/src/compiler/instructi...
src/compiler/instruction-selector-impl.h:158: from_location,
static_cast<MachineType>(kTypeAny | kMachPtr)));
On 2015/11/20 10:56:09, Jarin wrote:
> Why does not this use the incoming type? As suggested in
> instruction-selector.cc, we should perhaps rename the function to
> UsePointerLocation and hardcode kMachPtr here.

Done.

https://codereview.chromium.org/1455833004/diff/140001/src/compiler/instructi...
File src/compiler/instruction-selector.cc (right):

https://codereview.chromium.org/1455833004/diff/140001/src/compiler/instructi...
src/compiler/instruction-selector.cc:521: saved_return_location, kMachPtr);
On 2015/11/20 10:56:09, Jarin wrote:
> It looks like the UseLocation method is only called with kMachPtr. Perhaps you
> could hardcode kMachPtr there?

Done.

[danno, 2015-11-20 14:52:44]

The CQ bit was checked by danno@chromium.org

[danno, 2015-11-20 14:52:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmeurer@chromium.org, jarin@chromium.org
Link to the patchset: https://codereview.chromium.org/1455833004/#ps180001
(title: "Improve comment")

[commit-bot: I haz the power, 2015-11-20 14:52:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1455833004/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1455833004/180001

[commit-bot: I haz the power, 2015-11-20 15:03:53]

Committed patchset #11 (id:180001)

[commit-bot: I haz the power, 2015-11-20 15:04:39]

Patchset 11 (id:??) landed as
https://crrev.com/c6d310da4d86ce2d8178a49f4d34b66ad6143432
Cr-Commit-Position: refs/heads/master@{#32151}