[turbofan] Introduces the SpeculativeNumberShiftLeft opcode.

[turbofan] Introduces the SpeculativeNumberShiftLeft opcode.

Typed lowering now produces SpeculativeNumberShiftLeft for JSShiftLeft if the type feedback is kSignedSmall or kSigned32.

BUG=v8:4583
LOG=n

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

[epertoso, 2016-07-14 09:10:13]

Patchset #1 (id:1) has been deleted

[epertoso, 2016-07-14 09:23:56]

Description was changed from

==========
[WIP] Introduces the SpeculativeShiftLeft opcode.

BUG=
==========

to

==========
[turbofan] Introduces the SpeculativeNumberShiftLeft opcode.

Typed lowering now produces SpeculativeNumberShiftLeft for JSShiftLeft if the
type feedback is kSignedSmall or kSigned32.

BUG=v8:4583
LOG=n
==========

[epertoso, 2016-07-14 09:23:56]

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

[epertoso, 2016-07-14 09:23:56]

epertoso@chromium.org changed required reviewers:
+ jarin@chromium.org

[Jarin, 2016-07-14 10:27:58]

Looking good with some questions.

https://codereview.chromium.org/2150553002/diff/20001/src/compiler/js-typed-l...
File src/compiler/js-typed-lowering.cc (right):

https://codereview.chromium.org/2150553002/diff/20001/src/compiler/js-typed-l...
src/compiler/js-typed-lowering.cc:600: r.ConvertInputsToNumber();
We should only convert to number if the inputs are plain primitive. Otherwise,
we should return NoChange. (Just like we do in ReduceJSMultiply.)

https://codereview.chromium.org/2150553002/diff/20001/src/compiler/js-typed-l...
src/compiler/js-typed-lowering.cc:1960: return ReduceUI32ShiftLeft(node);
This looks like the signed version, so UI32 is misleading. How about
ReduceUI32ShiftLeft -> ReduceShiftLeft (or ReduceJSShiftLeft)?

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

https://codereview.chromium.org/2150553002/diff/20001/src/compiler/simplified...
src/compiler/simplified-lowering.cc:1564: if (BothInputsAre(node,
Type::Integral32())) {
Is not it enough if both inputs are numbers (Type::Number)? Or maybe even
NumberOrOddball?

https://codereview.chromium.org/2150553002/diff/20001/src/compiler/simplified...
src/compiler/simplified-lowering.cc:3282: }
Maybe we can factor this out to a separate function (now that
ReplaceEffectControlUses is not a method anymore).

[epertoso, 2016-07-14 12:02:12]

https://codereview.chromium.org/2150553002/diff/20001/src/compiler/js-typed-l...
File src/compiler/js-typed-lowering.cc (right):

https://codereview.chromium.org/2150553002/diff/20001/src/compiler/js-typed-l...
src/compiler/js-typed-lowering.cc:600: r.ConvertInputsToNumber();
On 2016/07/14 10:27:58, Jarin wrote:
> We should only convert to number if the inputs are plain primitive. Otherwise,
> we should return NoChange. (Just like we do in ReduceJSMultiply.)

Done.

https://codereview.chromium.org/2150553002/diff/20001/src/compiler/js-typed-l...
src/compiler/js-typed-lowering.cc:1960: return ReduceUI32ShiftLeft(node);
On 2016/07/14 10:27:58, Jarin wrote:
> This looks like the signed version, so UI32 is misleading. How about
> ReduceUI32ShiftLeft -> ReduceShiftLeft (or ReduceJSShiftLeft)?

Done.

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

https://codereview.chromium.org/2150553002/diff/20001/src/compiler/simplified...
src/compiler/simplified-lowering.cc:1564: if (BothInputsAre(node,
Type::Integral32())) {
On 2016/07/14 10:27:58, Jarin wrote:
> Is not it enough if both inputs are numbers (Type::Number)? Or maybe even
> NumberOrOddball?

Done.

https://codereview.chromium.org/2150553002/diff/20001/src/compiler/simplified...
src/compiler/simplified-lowering.cc:3282: }
On 2016/07/14 10:27:58, Jarin wrote:
> Maybe we can factor this out to a separate function (now that
> ReplaceEffectControlUses is not a method anymore).

Done.

[Jarin, 2016-07-14 12:15:38]

lgtm, thanks!

[epertoso, 2016-07-14 12:18:39]

The CQ bit was checked by epertoso@chromium.org

[commit-bot: I haz the power, 2016-07-14 12:18:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-14 12:47:14]

Description was changed from

==========
[turbofan] Introduces the SpeculativeNumberShiftLeft opcode.

Typed lowering now produces SpeculativeNumberShiftLeft for JSShiftLeft if the
type feedback is kSignedSmall or kSigned32.

BUG=v8:4583
LOG=n
==========

to

==========
[turbofan] Introduces the SpeculativeNumberShiftLeft opcode.

Typed lowering now produces SpeculativeNumberShiftLeft for JSShiftLeft if the
type feedback is kSignedSmall or kSigned32.

BUG=v8:4583
LOG=n
==========

[commit-bot: I haz the power, 2016-07-14 12:47:15]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-07-14 12:47:17]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-14 12:49:36]

Description was changed from

==========
[turbofan] Introduces the SpeculativeNumberShiftLeft opcode.

Typed lowering now produces SpeculativeNumberShiftLeft for JSShiftLeft if the
type feedback is kSignedSmall or kSigned32.

BUG=v8:4583
LOG=n
==========

to

==========
[turbofan] Introduces the SpeculativeNumberShiftLeft opcode.

Typed lowering now produces SpeculativeNumberShiftLeft for JSShiftLeft if the
type feedback is kSignedSmall or kSigned32.

BUG=v8:4583
LOG=n

Committed: https://crrev.com/d93fd41aaa0982ae8c4226cce3f2629649ebe87a
Cr-Commit-Position: refs/heads/master@{#37762}
==========

[commit-bot: I haz the power, 2016-07-14 12:49:37]

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