Detect simple tail calls

Detect simple tail calls

This CL contains the first steps towards tail call optimization:

  * Structurally detect tail calls during instruction selection,
    looking for special return/call combinations.

  * Added new architecture-specific instructions for tail calls which
    jump instead of call and take care of frame adjustment.

  * Moved some code around.

Currently we restrict tail calls to callees which only use registers
for arguments/return value and to call sites which are explicitly
marked as being OK for tail calls. This excludes, among other things,
call sites in sloppy JS functions and our IC machinery (both need in
general to be able to access the caller's frame).

All this is behind a flag --turbo-tail-calls, which is currently off
by default, so it can easily be toggled.

Committed: https://crrev.com/4b122b7504ab458faa4132c338809b40147751bf
Cr-Commit-Position: refs/heads/master@{#28150}

[Sven Panne, 2015-04-28 12:00:03]

svenpanne@chromium.org changed reviewers:
+ bmeurer@chromium.org

[Sven Panne, 2015-04-28 12:00:04]

Currently x64 port only and no test, just to get an initial round of review
comments.

[Benedikt Meurer, 2015-04-29 04:06:06]

https://codereview.chromium.org/1108563002/diff/60001/src/compiler/instructio...
File src/compiler/instruction-selector.cc (right):

https://codereview.chromium.org/1108563002/diff/60001/src/compiler/instructio...
src/compiler/instruction-selector.cc:1046: static Node* DetectTailCall(Node*
ret) {
Nit: Use anonymous namespace instead of static, and rename to something like
TryMatchTailCall.

https://codereview.chromium.org/1108563002/diff/60001/src/compiler/instructio...
src/compiler/instruction-selector.cc:1062: ?
(NodeProperties::GetControlInput(control, 0) == call)
Urghs, can we please not nest tenary operators?

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

https://codereview.chromium.org/1108563002/diff/60001/src/compiler/linkage.h#...
src/compiler/linkage.h:68: kIsTailCallAllowed = 1u << 4,
Can we rename this to kSupportsTailCalls?

https://codereview.chromium.org/1108563002/diff/60001/src/compiler/linkage.h#...
src/compiler/linkage.h:116: bool IsTailCallAllowed() const { return flags() &
kIsTailCallAllowed; }
SupportsTailCalls?

https://codereview.chromium.org/1108563002/diff/60001/src/compiler/x64/code-g...
File src/compiler/x64/code-generator-x64.cc (right):

https://codereview.chromium.org/1108563002/diff/60001/src/compiler/x64/code-g...
src/compiler/x64/code-generator-x64.cc:530: void
CodeGenerator::FixFrameForTailCall() {
Hm, FixFrameForTailCall is not an appropriate name. How about
AssembleDeconstructActivationRecord?

https://codereview.chromium.org/1108563002/diff/60001/src/compiler/x64/code-g...
src/compiler/x64/code-generator-x64.cc:541: __ popq(Operand(rsp, bytes_to_pop));
Had to close my eyes to review these lines of code...

[Sven Panne, 2015-04-29 10:24:18]

Feedback addressed, platform ports added, PTAL...

https://codereview.chromium.org/1108563002/diff/60001/src/compiler/instructio...
File src/compiler/instruction-selector.cc (right):

https://codereview.chromium.org/1108563002/diff/60001/src/compiler/instructio...
src/compiler/instruction-selector.cc:1046: static Node* DetectTailCall(Node*
ret) {
On 2015/04/29 04:06:05, Benedikt Meurer wrote:
> Nit: Use anonymous namespace instead of static, and rename to something like
> TryMatchTailCall.

Done.

https://codereview.chromium.org/1108563002/diff/60001/src/compiler/instructio...
src/compiler/instruction-selector.cc:1062: ?
(NodeProperties::GetControlInput(control, 0) == call)
On 2015/04/29 04:06:05, Benedikt Meurer wrote:
> Urghs, can we please not nest tenary operators?

Booh! Functional programming FTW! ;-) Nevertheless: Done... :-/

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

https://codereview.chromium.org/1108563002/diff/60001/src/compiler/linkage.h#...
src/compiler/linkage.h:68: kIsTailCallAllowed = 1u << 4,
On 2015/04/29 04:06:05, Benedikt Meurer wrote:
> Can we rename this to kSupportsTailCalls?

Done.

https://codereview.chromium.org/1108563002/diff/60001/src/compiler/linkage.h#...
src/compiler/linkage.h:116: bool IsTailCallAllowed() const { return flags() &
kIsTailCallAllowed; }
On 2015/04/29 04:06:05, Benedikt Meurer wrote:
> SupportsTailCalls?

Done.

https://codereview.chromium.org/1108563002/diff/60001/src/compiler/x64/code-g...
File src/compiler/x64/code-generator-x64.cc (right):

https://codereview.chromium.org/1108563002/diff/60001/src/compiler/x64/code-g...
src/compiler/x64/code-generator-x64.cc:530: void
CodeGenerator::FixFrameForTailCall() {
On 2015/04/29 04:06:05, Benedikt Meurer wrote:
> Hm, FixFrameForTailCall is not an appropriate name. How about
> AssembleDeconstructActivationRecord?

Done.

https://codereview.chromium.org/1108563002/diff/60001/src/compiler/x64/code-g...
src/compiler/x64/code-generator-x64.cc:541: __ popq(Operand(rsp, bytes_to_pop));
On 2015/04/29 04:06:05, Benedikt Meurer wrote:
> Had to close my eyes to review these lines of code...

... and I had to close my eyes to write them. :-D That's the beauty of our
current calling conventions...

[Benedikt Meurer, 2015-04-29 10:27:07]

How about some tests?

[Sven Panne, 2015-04-29 13:39:51]

Test added, PTAL...

[Benedikt Meurer, 2015-04-30 07:33:21]

LGTM

[Sven Panne, 2015-04-30 08:49:51]

The CQ bit was checked by svenpanne@chromium.org

[Sven Panne, 2015-04-30 08:49:51]

The patchset sent to the CQ was uploaded after l-g-t-m from bmeurer@chromium.org
Link to the patchset: https://codereview.chromium.org/1108563002/#ps170001
(title: "Fixed index type")

[commit-bot: I haz the power, 2015-04-30 08:50:03]

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

[commit-bot: I haz the power, 2015-04-30 09:10:27]

Committed patchset #10 (id:170001)

[commit-bot: I haz the power, 2015-04-30 09:10:41]

Patchset 10 (id:??) landed as
https://crrev.com/4b122b7504ab458faa4132c338809b40147751bf
Cr-Commit-Position: refs/heads/master@{#28150}