[wasm] Unittest for Int64Lowering.

[wasm] Unittest for Int64Lowering.

R=titzer@chromium.org

Committed: https://crrev.com/4ddf252e85deca1102e61b9245e33f873c099fb1
Cr-Commit-Position: refs/heads/master@{#34213}

[titzer, 2016-02-21 12:50:29]

https://codereview.chromium.org/1714793003/diff/1/test/unittests/compiler/int...
File test/unittests/compiler/int64-lowering-unittest.cc (right):

https://codereview.chromium.org/1714793003/diff/1/test/unittests/compiler/int...
test/unittests/compiler/int64-lowering-unittest.cc:57:
EXPECT_THAT(ret->InputAt(0)->opcode(), IrOpcode::kInt32Constant);
Please have a look at some of the other unittests. It's possible to build a
whole subgraph of expectations instead of checking each input individually.

[ahaas, 2016-02-22 15:19:41]

On 2016/02/21 at 12:50:29, titzer wrote:
>
https://codereview.chromium.org/1714793003/diff/1/test/unittests/compiler/int...
> File test/unittests/compiler/int64-lowering-unittest.cc (right):
> 
>
https://codereview.chromium.org/1714793003/diff/1/test/unittests/compiler/int...
> test/unittests/compiler/int64-lowering-unittest.cc:57:
EXPECT_THAT(ret->InputAt(0)->opcode(), IrOpcode::kInt32Constant);
> Please have a look at some of the other unittests. It's possible to build a
whole subgraph of expectations instead of checking each input individually.

Done.

I added tests for the other operators which we already support in Int64Lowering.

[ahaas, 2016-02-22 15:28:11]

https://codereview.chromium.org/1714793003/diff/40001/src/wasm/wasm-module.h
File src/wasm/wasm-module.h (right):

https://codereview.chromium.org/1714793003/diff/40001/src/wasm/wasm-module.h#...
src/wasm/wasm-module.h:210: static compiler::CallDescriptor*
GetWasmCallDescriptor(Zone* zone,
I don't see a reason why this function should not be static.

[ahaas, 2016-02-23 09:00:47]

ahaas@chromium.org changed reviewers:
+ mstarzinger@chromium.org

[Michael Starzinger, 2016-02-23 14:24:03]

LGTM to unblock you. I have to admit that I only skimmed over the
int64-lowering-unittest.cc file.

https://codereview.chromium.org/1714793003/diff/80001/test/unittests/compiler...
File test/unittests/compiler/int64-lowering-unittest.cc (right):

https://codereview.chromium.org/1714793003/diff/80001/test/unittests/compiler...
test/unittests/compiler/int64-lowering-unittest.cc:241:
IsReturn2(IsProjection(0, call_matcher),
If you wanted to make sure that both projections actually project out of the
"same" call node, you could use the capture mechanism. Just grep for
"testing::Capture" in the unittests directory to see some examples of how this
can be used. Just a suggestion, it's up to you.

https://codereview.chromium.org/1714793003/diff/80001/test/unittests/compiler...
File test/unittests/compiler/node-test-utils.cc (right):

https://codereview.chromium.org/1714793003/diff/80001/test/unittests/compiler...
test/unittests/compiler/node-test-utils.cc:326: value2_matcher_(value_matcher), 
// Just for initialization.
nit: Would it be possible to just use "_" matcher here?

[ahaas, 2016-02-23 14:48:56]

https://codereview.chromium.org/1714793003/diff/80001/test/unittests/compiler...
File test/unittests/compiler/int64-lowering-unittest.cc (right):

https://codereview.chromium.org/1714793003/diff/80001/test/unittests/compiler...
test/unittests/compiler/int64-lowering-unittest.cc:241:
IsReturn2(IsProjection(0, call_matcher),
On 2016/02/23 at 14:24:03, Michael Starzinger wrote:
> If you wanted to make sure that both projections actually project out of the
"same" call node, you could use the capture mechanism. Just grep for
"testing::Capture" in the unittests directory to see some examples of how this
can be used. Just a suggestion, it's up to you.

Done.

https://codereview.chromium.org/1714793003/diff/80001/test/unittests/compiler...
File test/unittests/compiler/node-test-utils.cc (right):

https://codereview.chromium.org/1714793003/diff/80001/test/unittests/compiler...
test/unittests/compiler/node-test-utils.cc:326: value2_matcher_(value_matcher), 
// Just for initialization.
On 2016/02/23 at 14:24:03, Michael Starzinger wrote:
> nit: Would it be possible to just use "_" matcher here?

Done.

[ahaas, 2016-02-23 15:24:13]

The CQ bit was checked by ahaas@chromium.org

[ahaas, 2016-02-23 15:24:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/1714793003/#ps100001
(title: "Michael's remarks")

[commit-bot: I haz the power, 2016-02-23 15:24:19]

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

[commit-bot: I haz the power, 2016-02-23 15:33:10]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-02-23 15:33:20]

Description was changed from

==========
[wasm] Unittest for Int64Lowering.

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

to

==========
[wasm] Unittest for Int64Lowering.

R=titzer@chromium.org

Committed: https://crrev.com/4ddf252e85deca1102e61b9245e33f873c099fb1
Cr-Commit-Position: refs/heads/master@{#34213}
==========

[commit-bot: I haz the power, 2016-02-23 15:33:22]

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