Adding support for multiple returns in compiled functions.

Adding support for multiple returns in compiled functions.

This will allow exploration of possibilities like passing around buffer base and length.

BUG=None
TEST=test-multiple-return
LOG=N
R=mtrofin@chromium.org,titzer@chromium.org

Committed: https://crrev.com/2e3c36fb1c9d1c99f7630fecee74e1fa2cfcd7c7
Cr-Commit-Position: refs/heads/master@{#31184}

[bradn, 2015-10-07 23:10:10]

Not quite ready. Sending out to mtrofin can help me figure out why register
allocation is blowing up on the caller function.

[titzer, 2015-10-07 23:57:05]

looks good modulo small comments.

https://codereview.chromium.org/1391333003/diff/1/src/compiler/instruction-se...
File src/compiler/instruction-selector.cc (right):

https://codereview.chromium.org/1391333003/diff/1/src/compiler/instruction-se...
src/compiler/instruction-selector.cc:1023: linkage()->GetReturnType());
GetReturnType(i) ?

https://codereview.chromium.org/1391333003/diff/1/src/compiler/instruction-se...
File src/compiler/instruction-selector.h (right):

https://codereview.chromium.org/1391333003/diff/1/src/compiler/instruction-se...
src/compiler/instruction-selector.h:216: void VisitReturn(Node* input);
We should probably call this "ret", since it should represent a return node.

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

https://codereview.chromium.org/1391333003/diff/1/src/compiler/linkage.h#newc...
src/compiler/linkage.h:312: MachineType GetReturnType() const { return
incoming_->GetReturnType(0); }
This one too.

https://codereview.chromium.org/1391333003/diff/1/test/cctest/compiler/test-m...
File test/cctest/compiler/test-multiple-return.cc (right):

https://codereview.chromium.org/1391333003/diff/1/test/cctest/compiler/test-m...
test/cctest/compiler/test-multiple-return.cc:36:
locations.AddReturn(LinkageLocation::ForAnyRegister());
I think you'll need to specify exact registers to make this work.

[bradn, 2015-10-08 17:13:37]

https://codereview.chromium.org/1391333003/diff/1/src/compiler/instruction-se...
File src/compiler/instruction-selector.cc (right):

https://codereview.chromium.org/1391333003/diff/1/src/compiler/instruction-se...
src/compiler/instruction-selector.cc:1023: linkage()->GetReturnType());
On 2015/10/07 23:57:05, titzer wrote:
> GetReturnType(i) ?

Done.

https://codereview.chromium.org/1391333003/diff/1/src/compiler/instruction-se...
File src/compiler/instruction-selector.h (right):

https://codereview.chromium.org/1391333003/diff/1/src/compiler/instruction-se...
src/compiler/instruction-selector.h:216: void VisitReturn(Node* input);
On 2015/10/07 23:57:05, titzer wrote:
> We should probably call this "ret", since it should represent a return node.

Done.

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

https://codereview.chromium.org/1391333003/diff/1/src/compiler/linkage.h#newc...
src/compiler/linkage.h:312: MachineType GetReturnType() const { return
incoming_->GetReturnType(0); }
On 2015/10/07 23:57:05, titzer wrote:
> This one too.

Done.

https://codereview.chromium.org/1391333003/diff/1/test/cctest/compiler/test-m...
File test/cctest/compiler/test-multiple-return.cc (right):

https://codereview.chromium.org/1391333003/diff/1/test/cctest/compiler/test-m...
test/cctest/compiler/test-multiple-return.cc:36:
locations.AddReturn(LinkageLocation::ForAnyRegister());
On 2015/10/07 23:57:05, titzer wrote:
> I think you'll need to specify exact registers to make this work.

Done.

[bradn, 2015-10-08 17:13:43]

ptal

[titzer, 2015-10-08 17:18:17]

lgtm other than small comment

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

https://codereview.chromium.org/1391333003/diff/20001/src/compiler/instructio...
src/compiler/instruction-selector.cc:1020: DCHECK_NOT_NULL(ret->InputAt(i));
Superfluous DCHECK. We rely on the graph verifier to check the invariants we
expect, so that the rest of the code can be mostly DCHECK-free :-)

[bradn, 2015-10-08 17:20:02]

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

https://codereview.chromium.org/1391333003/diff/20001/src/compiler/instructio...
src/compiler/instruction-selector.cc:1020: DCHECK_NOT_NULL(ret->InputAt(i));
On 2015/10/08 17:18:17, titzer wrote:
> Superfluous DCHECK. We rely on the graph verifier to check the invariants we
> expect, so that the rest of the code can be mostly DCHECK-free :-)

Done.

[bradn, 2015-10-08 17:21:13]

The CQ bit was checked by bradnelson@google.com

[bradn, 2015-10-08 17:21:13]

The patchset sent to the CQ was uploaded after l-g-t-m from titzer@chromium.org
Link to the patchset: https://codereview.chromium.org/1391333003/#ps60001
(title: "drop DCHECK")

[commit-bot: I haz the power, 2015-10-08 17:21:28]

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

[commit-bot: I haz the power, 2015-10-08 17:32:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-08 17:32:37]

Try jobs failed on following builders:
  v8_linux_nodcheck_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel/build...)

[bradn, 2015-10-08 17:58:50]

The CQ bit was checked by bradnelson@google.com

[bradn, 2015-10-08 17:58:50]

The patchset sent to the CQ was uploaded after l-g-t-m from titzer@chromium.org
Link to the patchset: https://codereview.chromium.org/1391333003/#ps80001
(title: "fix cargo culted test")

[commit-bot: I haz the power, 2015-10-08 17:59:05]

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

[bradn, 2015-10-08 18:06:43]

The CQ bit was checked by bradnelson@google.com

[bradn, 2015-10-08 18:06:43]

The patchset sent to the CQ was uploaded after l-g-t-m from titzer@chromium.org
Link to the patchset: https://codereview.chromium.org/1391333003/#ps100001
(title: "formatting")

[commit-bot: I haz the power, 2015-10-08 18:07:01]

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

[commit-bot: I haz the power, 2015-10-08 18:27:36]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-10-08 18:27:57]

Patchset 6 (id:??) landed as
https://crrev.com/2e3c36fb1c9d1c99f7630fecee74e1fa2cfcd7c7
Cr-Commit-Position: refs/heads/master@{#31184}

[MTBrandyberry, 2015-10-08 21:37:53]

mbrandy@us.ibm.com changed reviewers:
+ mbrandy@us.ibm.com

[MTBrandyberry, 2015-10-08 21:37:54]

https://codereview.chromium.org/1391333003/diff/100001/test/cctest/compiler/t...
File test/cctest/compiler/test-multiple-return.cc (right):

https://codereview.chromium.org/1391333003/diff/100001/test/cctest/compiler/t...
test/cctest/compiler/test-multiple-return.cc:36:
locations.AddReturn(LinkageLocation::ForRegister(i));
The set of register eligible for assignment as return values here and just below
as parameters needs to be platform-specific.  This current version crashes on
PPC since register index 1 corresponds to the stack pointer.

[titzer, 2015-10-08 22:01:35]

On 2015/10/08 21:37:54, mtbrandyberry wrote:
>
https://codereview.chromium.org/1391333003/diff/100001/test/cctest/compiler/t...
> File test/cctest/compiler/test-multiple-return.cc (right):
> 
>
https://codereview.chromium.org/1391333003/diff/100001/test/cctest/compiler/t...
> test/cctest/compiler/test-multiple-return.cc:36:
> locations.AddReturn(LinkageLocation::ForRegister(i));
> The set of register eligible for assignment as return values here and just
below
> as parameters needs to be platform-specific.  This current version crashes on
> PPC since register index 1 corresponds to the stack pointer.

We should use the allocatable register set, which is not platform specific.

[bradn, 2015-10-08 22:30:48]

Sorry about that, will send out a fix soon.