[turbofan] Introduce simplified NumberBitwise{Or,Xor,And} operators.

[turbofan] Introduce simplified NumberBitwise{Or,Xor,And} operators.

Currently we still (mis)used some machine operators in typed lowering
(namely Word32Or, Word32Xor and Word32And). But these operators are
"polymorphic" in the signedness of their inputs and output, hence the
representation selection (and thereby simplified lowering) was unable to
figure out whether a bitwise operation that was seen would produce an
unsigned or a signed result. If such nodes also have frame state uses,
the only safe choice was float64, which was not only a lot less ideal,
but also the main cause of the for-in related deoptimizer loops.

Adding dedicated NumberBitwiseOr, NumberBitwiseAnd and NumberBitwiseXor
simplified operators not only gives us precise (and correct) typing for
the bitwise operations, but also allows us to actually verify the graph
properly after typed lowering.

Drive-by-fix: Remove the double-to-smi magic from the Deoptimizer, which
is responsible for various deopt-loops in TurboFan, and is no longer
needed with the addition of the NumberBitwise operators.

R=jarin@chromium.org

Committed: https://crrev.com/d08f9045a1ee0062ced514dcb7f8047d505fd957
Cr-Commit-Position: refs/heads/master@{#31594}

[Benedikt Meurer, 2015-10-27 08:11:14]

bmeurer@chromium.org changed reviewers:
+ jarin@chromium.org

[Benedikt Meurer, 2015-10-27 08:11:14]

PTAL

[Benedikt Meurer, 2015-10-27 08:30:15]

Description was changed from

==========
[turbofan] Introduce simplified NumberBitwise{Or,Xor,And} operators.

Currently we still (mis)used some machine operators in typed lowering
(namely Word32Or, Word32Xor and Word32And). But these operators are
"polymorphic" in the signedness of their inputs and output, hence the
representation selection (and thereby simplified lowering) was unable to
figure out whether a bitwise operation that was seen would produce an
unsigned or a signed result. If such nodes also have frame state uses,
the only safe choice was float64, which was not only a lot less ideal,
but also the main cause of the for-in related deoptimizer loops.

Adding dedicated NumberBitwiseOr, NumberBitwiseAnd and NumberBitwiseXor
simplified operators not only gives us precise (and correct) typing for
the bitwise operations, but also allows us to actually verify the graph
properly after typed lowering.

Drive-by-fix: Remove the double-to-smi magic from the Deoptimizer, which
is responsible for various deopt-loops in TurboFan, and is no longer
needed with the addition of the NumberBitwise operators.
==========

to

==========
[turbofan] Introduce simplified NumberBitwise{Or,Xor,And} operators.

Currently we still (mis)used some machine operators in typed lowering
(namely Word32Or, Word32Xor and Word32And). But these operators are
"polymorphic" in the signedness of their inputs and output, hence the
representation selection (and thereby simplified lowering) was unable to
figure out whether a bitwise operation that was seen would produce an
unsigned or a signed result. If such nodes also have frame state uses,
the only safe choice was float64, which was not only a lot less ideal,
but also the main cause of the for-in related deoptimizer loops.

Adding dedicated NumberBitwiseOr, NumberBitwiseAnd and NumberBitwiseXor
simplified operators not only gives us precise (and correct) typing for
the bitwise operations, but also allows us to actually verify the graph
properly after typed lowering.

Drive-by-fix: Remove the double-to-smi magic from the Deoptimizer, which
is responsible for various deopt-loops in TurboFan, and is no longer
needed with the addition of the NumberBitwise operators.

R=jarin@chromium.org
==========

[Jarin, 2015-10-27 08:30:42]

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

https://codereview.chromium.org/1422213002/diff/1/src/compiler/simplified-low...
src/compiler/simplified-lowering.cc:724: VisitBinop(node, kMachInt32,
kMachUint32, kMachInt32);
This seems wrong, why is the right use UInt32? I believe you can just use
VisitInt32Binop(node) here.

[Benedikt Meurer, 2015-10-27 08:31:32]

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

https://codereview.chromium.org/1422213002/diff/1/src/compiler/simplified-low...
src/compiler/simplified-lowering.cc:724: VisitBinop(node, kMachInt32,
kMachUint32, kMachInt32);
On 2015/10/27 08:30:42, Jarin wrote:
> This seems wrong, why is the right use UInt32? I believe you can just use
> VisitInt32Binop(node) here.

Done.

[Jarin, 2015-10-27 08:33:25]

lgtm

[Benedikt Meurer, 2015-10-27 08:33:33]

The CQ bit was checked by bmeurer@chromium.org

[commit-bot: I haz the power, 2015-10-27 08:33:36]

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

[Jarin, 2015-10-27 08:42:28]

https://codereview.chromium.org/1422213002/diff/40001/src/deoptimizer.cc
File src/deoptimizer.cc (right):

https://codereview.chromium.org/1422213002/diff/40001/src/deoptimizer.cc#newc...
src/deoptimizer.cc:2567:
Handle<Object>(isolate()->factory()->NewHeapNumber(double_value()));
FYI, this is not what the deoptimizer was doing originally here. We have always
been calling NewNumber.

[Benedikt Meurer, 2015-10-27 08:43:05]

The CQ bit was unchecked by bmeurer@chromium.org

[Benedikt Meurer, 2015-10-27 08:48:50]

https://codereview.chromium.org/1422213002/diff/40001/src/deoptimizer.cc
File src/deoptimizer.cc (right):

https://codereview.chromium.org/1422213002/diff/40001/src/deoptimizer.cc#newc...
src/deoptimizer.cc:2567:
Handle<Object>(isolate()->factory()->NewHeapNumber(double_value()));
As discussed offline, changing this back to NewNumber.

[Benedikt Meurer, 2015-10-27 08:48:54]

The CQ bit was checked by bmeurer@chromium.org

[Benedikt Meurer, 2015-10-27 08:48:55]

The patchset sent to the CQ was uploaded after l-g-t-m from jarin@chromium.org
Link to the patchset: https://codereview.chromium.org/1422213002/#ps60001
(title: "Use NewNumber instead of NewHeapNumber in the Deoptimizer.")

[commit-bot: I haz the power, 2015-10-27 08:49:06]

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

[commit-bot: I haz the power, 2015-10-27 09:07:59]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-10-27 09:08:25]

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