[turbofan] Improve representation selection for Smi checking.

[turbofan] Improve representation selection for Smi checking.

Rename the high-level operators CheckTaggedSigned to CheckSmi and
CheckTaggedPointer to CheckHeapObject, to better match the naming
convention (i.e. ObjectIsSmi and CheckSmi, ObjectIsString and
CheckString, etc.).

For lowering CheckSmi, always report TaggedSigned representation
and let the RepresentationChanger come up with a reasonable conversion
from whatever input representation to TaggedSigned. This way we no
longer insert the useless ChangeSomethingToTagged and then Smi check
the result sequences, i.e. mostly reduces the amount of useless code
being generated. But we also observe a few performance improvements
on some crypto benchmarks.

This would enable us to avoid the Smi canonicalization when going from
Float64 to Tagged completely and thus match the representation selection
of Crankshaft in many areas (which might reduce the amount of
polymorphism until we fix our object model).

A follow-up CL will do the same for CheckHeapObject.

BUG=v8:5267
R=jarin@chromium.org

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

[Benedikt Meurer, 2016-09-23 07:36:58]

The CQ bit was checked by bmeurer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-23 07:37:08]

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

[Benedikt Meurer, 2016-09-23 07:38:26]

Hey Jaro,

Here's the first step to reduce friction in the area of field representations
between the four different compilers and the builtins.
Please take a look.

Thanks,
Benedikt

[commit-bot: I haz the power, 2016-09-23 07:58:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-23 07:58:39]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_avx2_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng_trig...)

[Benedikt Meurer, 2016-09-23 08:08:12]

The CQ bit was checked by bmeurer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-23 08:08:22]

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

[Jarin, 2016-09-23 08:31:54]

Quasi lgtm.

https://codereview.chromium.org/2362173003/diff/1/src/compiler/effect-control...
File src/compiler/effect-control-linearizer.cc (right):

https://codereview.chromium.org/2362173003/diff/1/src/compiler/effect-control...
src/compiler/effect-control-linearizer.cc:1715:
common()->DeoptimizeUnless(DeoptimizeReason::kLostPrecision), check,
So why is the deopt reason here kLostPrecision and above it is kOverflow?

https://codereview.chromium.org/2362173003/diff/1/src/compiler/representation...
File src/compiler/representation-change.cc (right):

https://codereview.chromium.org/2362173003/diff/1/src/compiler/representation...
src/compiler/representation-change.cc:190: if
(output_type->Is(Type::SignedSmall())) {
Note: At some point we should compile away NumberConstant during representation
selection...

https://codereview.chromium.org/2362173003/diff/1/src/compiler/representation...
src/compiler/representation-change.cc:209: } else {
How about 

else if (use_info.type_check() == TypeCheckKind::kSignedSmall) {
  op = simplified()->CheckedUint32ToTaggedSigned();
} else {
  TypeError(...);
}

(Here and below.)

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

https://codereview.chromium.org/2362173003/diff/1/src/compiler/simplified-low...
src/compiler/simplified-lowering.cc:2131: VisitUnop(node, UseInfo::AnyTagged(),
MachineRepresentation::kTagged);
Move visit binop out of the if?

Can we set the representation to kTaggedPointer here?

[Benedikt Meurer, 2016-09-23 08:39:38]

https://codereview.chromium.org/2362173003/diff/1/src/compiler/effect-control...
File src/compiler/effect-control-linearizer.cc (right):

https://codereview.chromium.org/2362173003/diff/1/src/compiler/effect-control...
src/compiler/effect-control-linearizer.cc:1715:
common()->DeoptimizeUnless(DeoptimizeReason::kLostPrecision), check,
Copied your logic from above.

https://codereview.chromium.org/2362173003/diff/1/src/compiler/representation...
File src/compiler/representation-change.cc (right):

https://codereview.chromium.org/2362173003/diff/1/src/compiler/representation...
src/compiler/representation-change.cc:190: if
(output_type->Is(Type::SignedSmall())) {
On 2016/09/23 08:31:54, Jarin wrote:
> Note: At some point we should compile away NumberConstant during
representation
> selection...

Acknowledged.

https://codereview.chromium.org/2362173003/diff/1/src/compiler/representation...
src/compiler/representation-change.cc:209: } else {
On 2016/09/23 08:31:54, Jarin wrote:
> How about 
> 
> else if (use_info.type_check() == TypeCheckKind::kSignedSmall) {
>   op = simplified()->CheckedUint32ToTaggedSigned();
> } else {
>   TypeError(...);
> }
> 
> (Here and below.)

Done.

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

https://codereview.chromium.org/2362173003/diff/1/src/compiler/simplified-low...
src/compiler/simplified-lowering.cc:2131: VisitUnop(node, UseInfo::AnyTagged(),
MachineRepresentation::kTagged);
On 2016/09/23 08:31:54, Jarin wrote:
> Move visit binop out of the if?

Not possible (yet), due to InputCannotBe

> Can we set the representation to kTaggedPointer here?

Not yet, rest of this file blows up.

[Benedikt Meurer, 2016-09-23 08:40:19]

The CQ bit was checked by bmeurer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-23 08:40:30]

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

[Benedikt Meurer, 2016-09-23 08:40:51]

The CQ bit was unchecked by bmeurer@chromium.org

[Benedikt Meurer, 2016-09-23 08:40:54]

The CQ bit was checked by bmeurer@chromium.org

[Benedikt Meurer, 2016-09-23 08:40: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/2362173003/#ps40001
(title: "Address comments")

[commit-bot: I haz the power, 2016-09-23 08:41:00]

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

[Benedikt Meurer, 2016-09-23 10:32:02]

The CQ bit was unchecked by bmeurer@chromium.org

[Benedikt Meurer, 2016-09-23 10:32:44]

Description was changed from

==========
[turbofan] Improve representation selection for Smi checking.

Rename the high-level operators CheckTaggedSigned to CheckSmi and
CheckTaggedPointer to CheckHeapObject, to better match the naming
convention (i.e. ObjectIsSmi and CheckSmi, ObjectIsString and
CheckString, etc.).

For lowering CheckSmi, always report TaggedSigned representation
and let the RepresentationChanger come up with a reasonable conversion
from whatever input representation to TaggedSigned. This way we no
longer insert the useless ChangeSomethingToTagged and then Smi check
the result sequences, i.e. mostly reduces the amount of useless code
being generated. But we also observe a few performance improvements
on some crypto benchmarks.

This would enable us to avoid the Smi canonicalization when going from
Float64 to Tagged completely and thus match the representation selection
of Crankshaft in many areas (which might reduce the amount of
polymorphism until we fix our object model).

A follow-up CL will do the same for CheckHeapObject.

BUG=v8:5267
R=jarin@chromium.org
==========

to

==========
[turbofan] Improve representation selection for Smi checking.

Rename the high-level operators CheckTaggedSigned to CheckSmi and
CheckTaggedPointer to CheckHeapObject, to better match the naming
convention (i.e. ObjectIsSmi and CheckSmi, ObjectIsString and
CheckString, etc.).

For lowering CheckSmi, always report TaggedSigned representation
and let the RepresentationChanger come up with a reasonable conversion
from whatever input representation to TaggedSigned. This way we no
longer insert the useless ChangeSomethingToTagged and then Smi check
the result sequences, i.e. mostly reduces the amount of useless code
being generated. But we also observe a few performance improvements
on some crypto benchmarks.

This would enable us to avoid the Smi canonicalization when going from
Float64 to Tagged completely and thus match the representation selection
of Crankshaft in many areas (which might reduce the amount of
polymorphism until we fix our object model).

A follow-up CL will do the same for CheckHeapObject.

BUG=v8:5267
R=jarin@chromium.org

Committed:
https://chromium.googlesource.com/v8/v8/+/dfbb3db5ef3401248177829eff4f36b9e0c...
==========

[commit-bot: I haz the power, 2016-09-23 10:32:44]

Description was changed from

==========
[turbofan] Improve representation selection for Smi checking.

Rename the high-level operators CheckTaggedSigned to CheckSmi and
CheckTaggedPointer to CheckHeapObject, to better match the naming
convention (i.e. ObjectIsSmi and CheckSmi, ObjectIsString and
CheckString, etc.).

For lowering CheckSmi, always report TaggedSigned representation
and let the RepresentationChanger come up with a reasonable conversion
from whatever input representation to TaggedSigned. This way we no
longer insert the useless ChangeSomethingToTagged and then Smi check
the result sequences, i.e. mostly reduces the amount of useless code
being generated. But we also observe a few performance improvements
on some crypto benchmarks.

This would enable us to avoid the Smi canonicalization when going from
Float64 to Tagged completely and thus match the representation selection
of Crankshaft in many areas (which might reduce the amount of
polymorphism until we fix our object model).

A follow-up CL will do the same for CheckHeapObject.

BUG=v8:5267
R=jarin@chromium.org
==========

to

==========
[turbofan] Improve representation selection for Smi checking.

Rename the high-level operators CheckTaggedSigned to CheckSmi and
CheckTaggedPointer to CheckHeapObject, to better match the naming
convention (i.e. ObjectIsSmi and CheckSmi, ObjectIsString and
CheckString, etc.).

For lowering CheckSmi, always report TaggedSigned representation
and let the RepresentationChanger come up with a reasonable conversion
from whatever input representation to TaggedSigned. This way we no
longer insert the useless ChangeSomethingToTagged and then Smi check
the result sequences, i.e. mostly reduces the amount of useless code
being generated. But we also observe a few performance improvements
on some crypto benchmarks.

This would enable us to avoid the Smi canonicalization when going from
Float64 to Tagged completely and thus match the representation selection
of Crankshaft in many areas (which might reduce the amount of
polymorphism until we fix our object model).

A follow-up CL will do the same for CheckHeapObject.

BUG=v8:5267
R=jarin@chromium.org

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

[commit-bot: I haz the power, 2016-09-23 10:32:45]

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

[Benedikt Meurer, 2016-09-23 10:32:45]

Committed patchset #3 (id:40001) manually as
dfbb3db5ef3401248177829eff4f36b9e0cc2324 (presubmit successful).