[wasm] Initial commit for the Int64Reducer.

[wasm] Initial commit for the Int64Reducer.

The goal of the Int64Reducer is to replace all int64 nodes in a tf graph
with a set of int32 nodes such that 64 bit tf functions can be executed
on 32 bit platforms. At the moment the Int64Reducer only replaces
Int64Constants, TruncateInt64ToInt32, and Word64And.

R=titzer@chromium.org

Committed: https://crrev.com/110173989818fff4b1fa5bebb09e56aa59ea9a6c
Cr-Commit-Position: refs/heads/master@{#33721}

[titzer, 2016-02-02 10:14:35]

https://codereview.chromium.org/1655883002/diff/20001/src/compiler/int64-redu...
File src/compiler/int64-reducer.cc (right):

https://codereview.chromium.org/1655883002/diff/20001/src/compiler/int64-redu...
src/compiler/int64-reducer.cc:29: for (;;) {
while (!stack.empty())

https://codereview.chromium.org/1655883002/diff/20001/src/compiler/int64-redu...
src/compiler/int64-reducer.cc:54: if (node->opcode() ==
IrOpcode::kInt64Constant) {
Make me a switch.

https://codereview.chromium.org/1655883002/diff/20001/src/compiler/int64-redu...
src/compiler/int64-reducer.cc:57:
graph()->NewNode(common()->Int32Constant(static_cast<int32_t>(value)));
I think you need to do a static_cast<uint32_t>(value & 0xFFFFFFFF) to be
absolutely sure to avoid C++ undefined behavior.

https://codereview.chromium.org/1655883002/diff/20001/src/compiler/int64-redu...
File src/compiler/int64-reducer.h (right):

https://codereview.chromium.org/1655883002/diff/20001/src/compiler/int64-redu...
src/compiler/int64-reducer.h:19: class Int64Reducer {
Let's call this Int64Lowering.

https://codereview.chromium.org/1655883002/diff/20001/src/compiler/wasm-compi...
File src/compiler/wasm-compiler.cc (right):

https://codereview.chromium.org/1655883002/diff/20001/src/compiler/wasm-compi...
src/compiler/wasm-compiler.cc:535: case wasm::kExprI64And:
Move this instead of adding more #ifdefs

https://codereview.chromium.org/1655883002/diff/20001/src/wasm/wasm-opcodes.cc
File src/wasm/wasm-opcodes.cc (right):

https://codereview.chromium.org/1655883002/diff/20001/src/wasm/wasm-opcodes.c...
src/wasm/wasm-opcodes.cc:100: // case kExprI64And:
Just remove them

https://codereview.chromium.org/1655883002/diff/20001/test/cctest/wasm/wasm-r...
File test/cctest/wasm/wasm-run-utils.h (right):

https://codereview.chromium.org/1655883002/diff/20001/test/cctest/wasm/wasm-r...
test/cctest/wasm/wasm-run-utils.h:289: Int64Reducer r(this->graph(),
this->machine(), this->common(),
I think you should move this use of the pipeline into a method in
wasm-compiler.cc and only do it if there is a node in the graph that requires
it. E.g. a WasmCompiler::CompileForTesting() method.

[ahaas, 2016-02-02 13:32:48]

https://codereview.chromium.org/1655883002/diff/20001/src/compiler/int64-redu...
File src/compiler/int64-reducer.cc (right):

https://codereview.chromium.org/1655883002/diff/20001/src/compiler/int64-redu...
src/compiler/int64-reducer.cc:29: for (;;) {
On 2016/02/02 at 10:14:35, titzer wrote:
> while (!stack.empty())

Done.

https://codereview.chromium.org/1655883002/diff/20001/src/compiler/int64-redu...
src/compiler/int64-reducer.cc:54: if (node->opcode() ==
IrOpcode::kInt64Constant) {
On 2016/02/02 at 10:14:35, titzer wrote:
> Make me a switch.

Done.

https://codereview.chromium.org/1655883002/diff/20001/src/compiler/int64-redu...
src/compiler/int64-reducer.cc:57:
graph()->NewNode(common()->Int32Constant(static_cast<int32_t>(value)));
On 2016/02/02 at 10:14:35, titzer wrote:
> I think you need to do a static_cast<uint32_t>(value & 0xFFFFFFFF) to be
absolutely sure to avoid C++ undefined behavior.

Done.

https://codereview.chromium.org/1655883002/diff/20001/src/compiler/wasm-compi...
File src/compiler/wasm-compiler.cc (right):

https://codereview.chromium.org/1655883002/diff/20001/src/compiler/wasm-compi...
src/compiler/wasm-compiler.cc:535: case wasm::kExprI64And:
On 2016/02/02 at 10:14:35, titzer wrote:
> Move this instead of adding more #ifdefs

Done.

https://codereview.chromium.org/1655883002/diff/20001/src/wasm/wasm-opcodes.cc
File src/wasm/wasm-opcodes.cc (right):

https://codereview.chromium.org/1655883002/diff/20001/src/wasm/wasm-opcodes.c...
src/wasm/wasm-opcodes.cc:100: // case kExprI64And:
On 2016/02/02 at 10:14:35, titzer wrote:
> Just remove them
Apparently some tests fail if I remove the instructions here already. We have to
wait until locals are supported (and maybe other stuff like function calls).

https://codereview.chromium.org/1655883002/diff/20001/test/cctest/wasm/wasm-r...
File test/cctest/wasm/wasm-run-utils.h (right):

https://codereview.chromium.org/1655883002/diff/20001/test/cctest/wasm/wasm-r...
test/cctest/wasm/wasm-run-utils.h:289: Int64Reducer r(this->graph(),
this->machine(), this->common(),
On 2016/02/02 at 10:14:35, titzer wrote:
> I think you should move this use of the pipeline into a method in
wasm-compiler.cc and only do it if there is a node in the graph that requires
it. E.g. a WasmCompiler::CompileForTesting() method.

I call WasmCompiler::Int64LoweringForTesting now after BuildTFGraph.

[titzer, 2016-02-03 15:36:29]

lgtm if below comments are addressed

https://codereview.chromium.org/1655883002/diff/40001/src/compiler/int64-lowe...
File src/compiler/int64-lowering.cc (right):

https://codereview.chromium.org/1655883002/diff/40001/src/compiler/int64-lowe...
src/compiler/int64-lowering.cc:23:
replacements_(zone->NewArray<Replacement>(graph->NodeCount())) {}
Maybe we should zero-initialize the replacements as well?

https://codereview.chromium.org/1655883002/diff/40001/test/cctest/wasm/wasm-r...
File test/cctest/wasm/wasm-run-utils.h (right):

https://codereview.chromium.org/1655883002/diff/40001/test/cctest/wasm/wasm-r...
test/cctest/wasm/wasm-run-utils.h:240: builder.Int64LoweringForTesting();
This wasn't really what I meant with my earlier comment, but it's OK. You can
just inline this now. We can make it nicer in a later CL.

[ahaas, 2016-02-03 15:48:40]

https://codereview.chromium.org/1655883002/diff/40001/src/compiler/int64-lowe...
File src/compiler/int64-lowering.cc (right):

https://codereview.chromium.org/1655883002/diff/40001/src/compiler/int64-lowe...
src/compiler/int64-lowering.cc:23:
replacements_(zone->NewArray<Replacement>(graph->NodeCount())) {}
On 2016/02/03 at 15:36:29, titzer wrote:
> Maybe we should zero-initialize the replacements as well?

I will do this in a later CL.

[ahaas, 2016-02-04 09:08:06]

https://codereview.chromium.org/1655883002/diff/40001/src/compiler/int64-lowe...
File src/compiler/int64-lowering.cc (right):

https://codereview.chromium.org/1655883002/diff/40001/src/compiler/int64-lowe...
src/compiler/int64-lowering.cc:23:
replacements_(zone->NewArray<Replacement>(graph->NodeCount())) {}
On 2016/02/03 at 15:48:40, ahaas wrote:
> On 2016/02/03 at 15:36:29, titzer wrote:
> > Maybe we should zero-initialize the replacements as well?
> 
> I will do this in a later CL.

Done

[ahaas, 2016-02-04 09:37:37]

The CQ bit was checked by ahaas@chromium.org

[ahaas, 2016-02-04 09:37:37]

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/1655883002/#ps80001
(title: "Initialize replacement array")

[commit-bot: I haz the power, 2016-02-04 09:37:46]

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

[commit-bot: I haz the power, 2016-02-04 09:40:58]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-02-04 09:41:31]

Description was changed from

==========
[wasm] Initial commit for the Int64Reducer.

The goal of the Int64Reducer is to replace all int64 nodes in a tf graph
with a set of int32 nodes such that 64 bit tf functions can be executed
on 32 bit platforms. At the moment the Int64Reducer only replaces
Int64Constants, TruncateInt64ToInt32, and Word64And.

R=titzer@chromium.org
==========

to

==========
[wasm] Initial commit for the Int64Reducer.

The goal of the Int64Reducer is to replace all int64 nodes in a tf graph
with a set of int32 nodes such that 64 bit tf functions can be executed
on 32 bit platforms. At the moment the Int64Reducer only replaces
Int64Constants, TruncateInt64ToInt32, and Word64And.

R=titzer@chromium.org

Committed: https://crrev.com/110173989818fff4b1fa5bebb09e56aa59ea9a6c
Cr-Commit-Position: refs/heads/master@{#33721}
==========

[commit-bot: I haz the power, 2016-02-04 09:41:31]

Patchset 5 (id:??) landed as
https://crrev.com/110173989818fff4b1fa5bebb09e56aa59ea9a6c
Cr-Commit-Position: refs/heads/master@{#33721}