Commit 12ce15c3 authored by tebbi's avatar tebbi Committed by Commit bot

[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}
parent 78867ad8
......@@ -886,7 +886,7 @@ class Builtins {
// Disassembler support.
const char* Lookup(byte* pc);
enum Name {
enum Name : int32_t {
#define DEF_ENUM(Name, ...) k##Name,
BUILTIN_LIST_ALL(DEF_ENUM)
#undef DEF_ENUM
......
......@@ -252,6 +252,9 @@ struct BinopMatcher : public NodeMatcher {
protected:
void SwapInputs() {
std::swap(left_, right_);
// TODO(tebbi): This modification should notify the reducers using
// BinopMatcher. Alternatively, all reducers (especially value numbering)
// could ignore the ordering for commutative binops.
node()->ReplaceInput(0, left().node());
node()->ReplaceInput(1, right().node());
}
......
......@@ -693,9 +693,9 @@ PipelineWasmCompilationJob::ExecuteJobImpl() {
CommonOperatorReducer common_reducer(&graph_reducer, data->graph(),
data->common(), data->machine());
AddReducer(data, &graph_reducer, &dead_code_elimination);
AddReducer(data, &graph_reducer, &value_numbering);
AddReducer(data, &graph_reducer, &machine_reducer);
AddReducer(data, &graph_reducer, &common_reducer);
AddReducer(data, &graph_reducer, &value_numbering);
graph_reducer.ReduceGraph();
pipeline_.RunPrintAndVerify("Optimized Machine", true);
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment