Lower NumberMultiply, NumberDivide, and NumberModulus to Int32Mul, Int32[U]Div, and Int32[U]Mod whe…

Lower NumberMultiply, NumberDivide, and NumberModulus to Int32Mul, Int32[U]Div, and Int32[U]Mod when possible in simplified-lowering.

R=mstarzinger@chromium.org
BUG=

Committed: https://code.google.com/p/v8/source/detail?r=24462

[Michael Starzinger, 2014-10-01 21:11:33]

Looking good. Please also add tests that actually run the generated code (as
discussed offline).

https://codereview.chromium.org/620553008/diff/1/src/compiler/simplified-lowe...
File src/compiler/simplified-lowering.cc (right):

https://codereview.chromium.org/620553008/diff/1/src/compiler/simplified-lowe...
src/compiler/simplified-lowering.cc:355: bool CanBeInt32Binop(Node* node,
MachineTypeUnion use) {
I find the "CanBe" prefix confusing in this setting, I assume it is meant to
indicate "can be lowered to"? Could we instead just call it "IsInt32Binop"?

https://codereview.chromium.org/620553008/diff/1/src/compiler/simplified-lowe...
src/compiler/simplified-lowering.cc:359: bool CanBeUint32Binop(Node* node,
MachineTypeUnion use) {
Likewise.

[Michael Starzinger, 2014-10-01 21:15:57]

https://codereview.chromium.org/620553008/diff/1/src/compiler/simplified-lowe...
File src/compiler/simplified-lowering.cc (right):

https://codereview.chromium.org/620553008/diff/1/src/compiler/simplified-lowe...
src/compiler/simplified-lowering.cc:355: bool CanBeInt32Binop(Node* node,
MachineTypeUnion use) {
On 2014/10/01 21:11:32, Michael Starzinger wrote:
> I find the "CanBe" prefix confusing in this setting, I assume it is meant to
> indicate "can be lowered to"? Could we instead just call it "IsInt32Binop"?

Alternatively "CanBecome" would also work for me.

[titzer, 2014-10-07 16:19:26]

Added tests. PTAL.

[Michael Starzinger, 2014-10-08 09:01:28]

LGTM (if all comments are addressed).

https://codereview.chromium.org/620553008/diff/20001/src/compiler/simplified-...
File src/compiler/simplified-lowering.cc (right):

https://codereview.chromium.org/620553008/diff/20001/src/compiler/simplified-...
src/compiler/simplified-lowering.cc:355: bool CanBeInt32Binop(Node* node,
MachineTypeUnion use) {
Please address my previous comment about the "CanBe" prefix. As discussed
offline, I would be fine with "CanLowerTo" instead.

https://codereview.chromium.org/620553008/diff/20001/test/cctest/compiler/tes...
File test/cctest/compiler/test-simplified-lowering.cc (right):

https://codereview.chromium.org/620553008/diff/20001/test/cctest/compiler/tes...
test/cctest/compiler/test-simplified-lowering.cc:62: FILE* dot_file =
fopen("/tmp/test-lowered.dot", "w+");
Looks like a left-over, let's drop it again.

https://codereview.chromium.org/620553008/diff/20001/test/mjsunit/asm/int32_d...
File test/mjsunit/asm/int32_div.js (right):

https://codereview.chromium.org/620553008/diff/20001/test/mjsunit/asm/int32_d...
test/mjsunit/asm/int32_div.js:1: // Copyright 2014 the V8 project authors. All
rights reserved.
Can we rename this file (and friends) to use dashes instead of underscores to be
consistent with the rest of the code-base?

[titzer, 2014-10-08 09:12:29]

https://codereview.chromium.org/620553008/diff/20001/src/compiler/simplified-...
File src/compiler/simplified-lowering.cc (right):

https://codereview.chromium.org/620553008/diff/20001/src/compiler/simplified-...
src/compiler/simplified-lowering.cc:355: bool CanBeInt32Binop(Node* node,
MachineTypeUnion use) {
On 2014/10/08 09:01:28, Michael Starzinger wrote:
> Please address my previous comment about the "CanBe" prefix. As discussed
> offline, I would be fine with "CanLowerTo" instead.

Done.

https://codereview.chromium.org/620553008/diff/20001/test/cctest/compiler/tes...
File test/cctest/compiler/test-simplified-lowering.cc (right):

https://codereview.chromium.org/620553008/diff/20001/test/cctest/compiler/tes...
test/cctest/compiler/test-simplified-lowering.cc:62: FILE* dot_file =
fopen("/tmp/test-lowered.dot", "w+");
On 2014/10/08 09:01:28, Michael Starzinger wrote:
> Looks like a left-over, let's drop it again.

Done.

https://codereview.chromium.org/620553008/diff/20001/test/mjsunit/asm/int32_d...
File test/mjsunit/asm/int32_div.js (right):

https://codereview.chromium.org/620553008/diff/20001/test/mjsunit/asm/int32_d...
test/mjsunit/asm/int32_div.js:1: // Copyright 2014 the V8 project authors. All
rights reserved.
On 2014/10/08 09:01:28, Michael Starzinger wrote:
> Can we rename this file (and friends) to use dashes instead of underscores to
be
> consistent with the rest of the code-base?

Done.

[titzer, 2014-10-08 10:54:00]

Committed patchset #4 manually as 24462 (presubmit successful).