[turbofan] Do not reduce Return nodes with multiple value inputs.

[turbofan] Do not reduce Return nodes with multiple value inputs.

The existing implementation assumes that return nodes have exactly one
real value input. This assumption does not hold for WebAssembly. To
avoid incorrect behavior, this CL turns of the reduction of returns
with a value input count != 1.

R=titzer@chromium.org, mstarzinger@chromium.org

Review-Url: https://codereview.chromium.org/2638053002
Cr-Commit-Position: refs/heads/master@{#42425}
Committed: https://chromium.googlesource.com/v8/v8/+/cfa6ce3232bb402ab9ecd9c201ab840560cbefcf

[ahaas, 2017-01-17 16:01:44]

The CQ bit was checked by ahaas@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-17 16:01:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[titzer, 2017-01-17 16:08:22]

lgtm if comment is added

https://codereview.chromium.org/2638053002/diff/1/src/compiler/common-operato...
File src/compiler/common-operator-reducer.cc (right):

https://codereview.chromium.org/2638053002/diff/1/src/compiler/common-operato...
src/compiler/common-operator-reducer.cc:300: if
(NodeProperties::PastValueIndex(node) != 2) {
This is worth a comment and a TODO. This is doing an optimization that turns a
return of a phi into two returns, afaict. We can generalize that later to also
work with multiple return values.

[commit-bot: I haz the power, 2017-01-17 16:35:03]

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

[commit-bot: I haz the power, 2017-01-17 16:35:04]

Dry run: This issue passed the CQ dry run.

[titzer, 2017-01-17 16:43:21]

https://codereview.chromium.org/2638053002/diff/1/src/compiler/common-operato...
File src/compiler/common-operator-reducer.cc (right):

https://codereview.chromium.org/2638053002/diff/1/src/compiler/common-operato...
src/compiler/common-operator-reducer.cc:300: if
(NodeProperties::PastValueIndex(node) != 2) {
Please also use the value input count to make this clearer.

[ahaas, 2017-01-17 17:30:38]

The CQ bit was checked by ahaas@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-17 17:30:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ahaas, 2017-01-17 17:35:54]

https://codereview.chromium.org/2638053002/diff/1/src/compiler/common-operato...
File src/compiler/common-operator-reducer.cc (right):

https://codereview.chromium.org/2638053002/diff/1/src/compiler/common-operato...
src/compiler/common-operator-reducer.cc:300: if
(NodeProperties::PastValueIndex(node) != 2) {
On 2017/01/17 at 16:08:22, titzer wrote:
> This is worth a comment and a TODO. This is doing an optimization that turns a
return of a phi into two returns, afaict. We can generalize that later to also
work with multiple return values.

I added the TODO.

PTAL: I added a helper function for return operators now which returns the value
input count of a return node without the helper input at index 0.

[titzer, 2017-01-17 17:46:00]

On 2017/01/17 17:35:54, ahaas wrote:
>
https://codereview.chromium.org/2638053002/diff/1/src/compiler/common-operato...
> File src/compiler/common-operator-reducer.cc (right):
> 
>
https://codereview.chromium.org/2638053002/diff/1/src/compiler/common-operato...
> src/compiler/common-operator-reducer.cc:300: if
> (NodeProperties::PastValueIndex(node) != 2) {
> On 2017/01/17 at 16:08:22, titzer wrote:
> > This is worth a comment and a TODO. This is doing an optimization that turns
a
> return of a phi into two returns, afaict. We can generalize that later to also
> work with multiple return values.
> 
> I added the TODO.
> 
> PTAL: I added a helper function for return operators now which returns the
value
> input count of a return node without the helper input at index 0.

LGTM

[ahaas, 2017-01-17 18:34:27]

The CQ bit was checked by ahaas@chromium.org

[commit-bot: I haz the power, 2017-01-17 18:34:39]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-17 19:01:33]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484678067679920,
"parent_rev": "5e31df230cfec26e4f374a8b8e0c2c7626cc1d92", "commit_rev":
"cfa6ce3232bb402ab9ecd9c201ab840560cbefcf"}

[commit-bot: I haz the power, 2017-01-17 19:01:37]

Description was changed from

==========
[turbofan] Do not reduce Return nodes with multiple value inputs.

The existing implementation assumes that return nodes have exactly one
real value input. This assumption does not hold for WebAssembly. To
avoid incorrect behavior, this CL turns of the reduction of returns
with a value input count != 1.

R=titzer@chromium.org, mstarzinger@chromium.org
==========

to

==========
[turbofan] Do not reduce Return nodes with multiple value inputs.

The existing implementation assumes that return nodes have exactly one
real value input. This assumption does not hold for WebAssembly. To
avoid incorrect behavior, this CL turns of the reduction of returns
with a value input count != 1.

R=titzer@chromium.org, mstarzinger@chromium.org

Review-Url: https://codereview.chromium.org/2638053002
Cr-Commit-Position: refs/heads/master@{#42425}
Committed:
https://chromium.googlesource.com/v8/v8/+/cfa6ce3232bb402ab9ecd9c201ab840560c...
==========

[commit-bot: I haz the power, 2017-01-17 19:01:38]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/v8/v8/+/cfa6ce3232bb402ab9ecd9c201ab840560c...