[wasm] change reducer order in WASM pipeline to make build predictable again

[wasm] change reducer order in WASM pipeline to make build predictable again

BinopMatcher does not notify the reducers using it when it flips inputs to commutative operators. This leads to value numbering not being re-executed in this case. Together with the fact that value numbering might still reduce such a modified node in the case of a hash collision merging the buckets of two equivalent nodes, this leads to unpredictable behaviour.

This is the easiest fix for the problem: Always running value numbering last. This is also a performance improvement because value numbering never changes but only replaces nodes.

R=mstarzinger@chromium.org

Review-Url: https://codereview.chromium.org/2728983002
Cr-Commit-Position: refs/heads/master@{#43552}
Committed: https://chromium.googlesource.com/v8/v8/+/12ce15c35b0b457cacc20d53718114969cdd7414

[Tobias Tebbi, 2017-03-02 10:28:38]

https://codereview.chromium.org/2728983002/diff/20001/src/builtins/builtins.h
File src/builtins/builtins.h (right):

https://codereview.chromium.org/2728983002/diff/20001/src/builtins/builtins.h...
src/builtins/builtins.h:889: enum Name : int32_t {
drive-by fix to recover the change from 
https://chromium-review.googlesource.com/445937
that got lost in a faulty rebase in
https://codereview.chromium.org/2692753004

[Michael Starzinger, 2017-03-02 16:38:36]

LGTM.

https://codereview.chromium.org/2728983002/diff/20001/src/builtins/builtins.h
File src/builtins/builtins.h (right):

https://codereview.chromium.org/2728983002/diff/20001/src/builtins/builtins.h...
src/builtins/builtins.h:889: enum Name : int32_t {
On 2017/03/02 10:28:38, Tobias Tebbi wrote:
> drive-by fix to recover the change from 
> https://chromium-review.googlesource.com/445937
> that got lost in a faulty rebase in
> https://codereview.chromium.org/2692753004

Acknowledged.

https://codereview.chromium.org/2728983002/diff/20001/src/compiler/node-match...
File src/compiler/node-matchers.h (right):

https://codereview.chromium.org/2728983002/diff/20001/src/compiler/node-match...
src/compiler/node-matchers.h:255: // TODO(tebbi): This modification should
notify the reducers using
It might make sense to also file a bug on the issue tracker regarding this.

[Tobias Tebbi, 2017-03-02 16:39:55]

The CQ bit was checked by tebbi@chromium.org

[commit-bot: I haz the power, 2017-03-02 16:40:03]

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-03-02 17:53:33]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1488472795467890,
"parent_rev": "78867ad8707a0163b27b925d9282f18fd4c026ae", "commit_rev":
"12ce15c35b0b457cacc20d53718114969cdd7414"}

[commit-bot: I haz the power, 2017-03-02 17:53:38]

Description was changed from

==========
[wasm] change reducer order in WASM pipeline to make build predictable again

BinopMatcher does not notify the reducers using it when it flips inputs to
commutative operators. This leads to value numbering not being re-executed in
this case. Together with the fact that value numbering might still reduce such a
modified node in the case of a hash collision merging the buckets of two
equivalent nodes, this leads to unpredictable behaviour.

This is the easiest fix for the problem: Always running value numbering last.
This is also a performance improvement because value numbering never changes but
only replaces nodes.

R=mstarzinger@chromium.org
==========

to

==========
[wasm] change reducer order in WASM pipeline to make build predictable again

BinopMatcher does not notify the reducers using it when it flips inputs to
commutative operators. This leads to value numbering not being re-executed in
this case. Together with the fact that value numbering might still reduce such a
modified node in the case of a hash collision merging the buckets of two
equivalent nodes, this leads to unpredictable behaviour.

This is the easiest fix for the problem: Always running value numbering last.
This is also a performance improvement because value numbering never changes but
only replaces nodes.

R=mstarzinger@chromium.org

Review-Url: https://codereview.chromium.org/2728983002
Cr-Commit-Position: refs/heads/master@{#43552}
Committed:
https://chromium.googlesource.com/v8/v8/+/12ce15c35b0b457cacc20d53718114969cd...
==========

[commit-bot: I haz the power, 2017-03-02 17:53:39]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/v8/v8/+/12ce15c35b0b457cacc20d53718114969cd...

[Tobias Tebbi, 2017-03-03 09:47:30]

Patchset #3 (id:40001) has been deleted