[Turbofan] Introduce OtherNumberConstant.

[Turbofan] Introduce OtherNumberConstant.

With this CL, we devolve all Constants introduced as they are with an object handle into

* Range - for integers
* Nan
* MinusZero
* OtherNumberConstant - for doubles
* HeapConstant

We reduce the amount we have to inspect an object handle during optimization. Also, simplifications result. For example, you never have to check if a Range contains a HeapConstant.

BUG=

Committed: https://crrev.com/978fe70beb3d297e34037d3776b39e9018d52980
Cr-Commit-Position: refs/heads/master@{#40041}

[mvstanton, 2016-09-28 14:47:44]

Address my comments.

https://codereview.chromium.org/2381523002/diff/20001/src/compiler/typer.cc
File src/compiler/typer.cc (right):

https://codereview.chromium.org/2381523002/diff/20001/src/compiler/typer.cc#n...
src/compiler/typer.cc:596: return Type::Range(number, number, zone());
Type::IsInteger... remove that! Factory covers it!

https://codereview.chromium.org/2381523002/diff/20001/src/compiler/types.cc
File src/compiler/types.cc (right):

https://codereview.chromium.org/2381523002/diff/20001/src/compiler/types.cc#n...
src/compiler/types.cc:67: return IsInteger(rhs->Value()) && lhs->Min() <=
rhs->Value() &&
Actually don't do this - a numberconstant is NEVER part of a Range!

https://codereview.chromium.org/2381523002/diff/20001/src/compiler/types.cc#n...
src/compiler/types.cc:698: return AddToUnion(rhs, result, size, zone);
leave this out, because a range cannot contain a numberconstant, intersection is
empty.

https://codereview.chromium.org/2381523002/diff/20001/src/compiler/types.cc#n...
src/compiler/types.cc:778: return Constant(value, zone);
drive by comment - shouldn't we canonicalize NaN and -0 constants? And the
bitsets for those should be correct.

https://codereview.chromium.org/2381523002/diff/20001/src/compiler/types.cc#n...
src/compiler/types.cc:779: }
Also, a separate CL to land after would make sure you never try to create a
Constant(-0) or Constant(NaN), instead, there should be a bitset for those
created.

BTW, this factory method can take care of that for us.

https://codereview.chromium.org/2381523002/diff/20001/src/compiler/types.h
File src/compiler/types.h (right):

https://codereview.chromium.org/2381523002/diff/20001/src/compiler/types.h#ne...
src/compiler/types.h:297: return !std::isnan(value) && nearbyint(value) != value
&&
make sure you don't allow integers outside of smi range!
try and use the IsInteger() predicate to exclude values.

https://codereview.chromium.org/2381523002/diff/20001/src/compiler/types.h#ne...
src/compiler/types.h:322: 
No need for bitset member.

https://codereview.chromium.org/2381523002/diff/20001/src/compiler/types.h#ne...
src/compiler/types.h:351: HeapNumber::cast(*object)->value()));
Confusing!

https://codereview.chromium.org/2381523002/diff/20001/test/cctest/test-types.cc
File test/cctest/test-types.cc (right):

https://codereview.chromium.org/2381523002/diff/20001/test/cctest/test-types....
test/cctest/test-types.cc:728: Type* const_type2 = T.NewConstant(value2);
This and other "rule-enforcement" tests need conditionals in case we deal with
OtherNumberConstants().

Also, add to the values array 2 different HeapNumbers with the same double
value.

And add a Smi and a boxed version of the Smi.

[mvstanton, 2016-10-04 11:55:54]

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

[mvstanton, 2016-10-04 11:55:55]

Hi Jaro,
Here is the code after our initial review at my desk.
Thanks!
--Mike

https://codereview.chromium.org/2381523002/diff/20001/src/compiler/typer.cc
File src/compiler/typer.cc (right):

https://codereview.chromium.org/2381523002/diff/20001/src/compiler/typer.cc#n...
src/compiler/typer.cc:596: return Type::Range(number, number, zone());
On 2016/09/28 14:47:44, mvstanton wrote:
> Type::IsInteger... remove that! Factory covers it!

Done.

https://codereview.chromium.org/2381523002/diff/20001/src/compiler/types.cc
File src/compiler/types.cc (right):

https://codereview.chromium.org/2381523002/diff/20001/src/compiler/types.cc#n...
src/compiler/types.cc:67: return IsInteger(rhs->Value()) && lhs->Min() <=
rhs->Value() &&
On 2016/09/28 14:47:44, mvstanton wrote:
> Actually don't do this - a numberconstant is NEVER part of a Range!

Done.

https://codereview.chromium.org/2381523002/diff/20001/src/compiler/types.cc#n...
src/compiler/types.cc:698: return AddToUnion(rhs, result, size, zone);
On 2016/09/28 14:47:44, mvstanton wrote:
> leave this out, because a range cannot contain a numberconstant, intersection
is
> empty.

Done.

https://codereview.chromium.org/2381523002/diff/20001/src/compiler/types.cc#n...
src/compiler/types.cc:778: return Constant(value, zone);
On 2016/09/28 14:47:44, mvstanton wrote:
> drive by comment - shouldn't we canonicalize NaN and -0 constants? And the
> bitsets for those should be correct.

Done.

https://codereview.chromium.org/2381523002/diff/20001/src/compiler/types.h
File src/compiler/types.h (right):

https://codereview.chromium.org/2381523002/diff/20001/src/compiler/types.h#ne...
src/compiler/types.h:297: return !std::isnan(value) && nearbyint(value) != value
&&
On 2016/09/28 14:47:44, mvstanton wrote:
> make sure you don't allow integers outside of smi range!
> try and use the IsInteger() predicate to exclude values.

Done.

https://codereview.chromium.org/2381523002/diff/20001/src/compiler/types.h#ne...
src/compiler/types.h:322: 
On 2016/09/28 14:47:44, mvstanton wrote:
> No need for bitset member.

Done.

https://codereview.chromium.org/2381523002/diff/20001/src/compiler/types.h#ne...
src/compiler/types.h:351: HeapNumber::cast(*object)->value()));
On 2016/09/28 14:47:44, mvstanton wrote:
> Confusing!

Done.

https://codereview.chromium.org/2381523002/diff/20001/test/cctest/test-types.cc
File test/cctest/test-types.cc (right):

https://codereview.chromium.org/2381523002/diff/20001/test/cctest/test-types....
test/cctest/test-types.cc:728: Type* const_type2 = T.NewConstant(value2);
On 2016/09/28 14:47:44, mvstanton wrote:
> This and other "rule-enforcement" tests need conditionals in case we deal with
> OtherNumberConstants().
> 
> Also, add to the values array 2 different HeapNumbers with the same double
> value.
> 
> And add a Smi and a boxed version of the Smi.

Done.

[Jarin, 2016-10-04 13:14:55]

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/typer.cc
File src/compiler/typer.cc (right):

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/typer.cc#n...
src/compiler/typer.cc:595: return Type::NewConstant(f->NewNumber(number),
zone());
How about having a factory from a double value?

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/types.cc
File src/compiler/types.cc (right):

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/types.cc#n...
src/compiler/types.cc:85: if (this->IsConstant()) return
this->AsConstant()->Value()->Number();
Remove this line, please (Constants should be only heap objects now).

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/types.cc#n...
src/compiler/types.cc:103: if (this->IsConstant()) return
this->AsConstant()->Value()->Number();
Remove.

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/types.h
File src/compiler/types.h (right):

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/types.h#ne...
src/compiler/types.h:301: return !std::isnan(value) && !IsInteger(value) &&
!i::IsMinusZero(value);
IsInteger => Type::IsInteger (and remove the IsInteger method above?)

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/types.h#ne...
src/compiler/types.h:306: return
IsOtherNumberConstant(HeapNumber::cast(value)->value());
Nit: 

return value->IsHeapNumber() &&
    IsOtherNumberConstant(HeapNumber::cast(value)->value());

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/types.h#ne...
src/compiler/types.h:335: class ConstantType : public TypeBase {
Maybe ConstantType => HeapConstantType?

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/types.h#ne...
src/compiler/types.h:527: static Type* Constant(i::Handle<i::Object> value,
Zone* zone) {
Maybe Constant => HeapConstant?

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/types.h#ne...
src/compiler/types.h:571: bool IsConstant() { return
IsKind(TypeBase::kConstant); }
IsHeapConstant?

[mvstanton, 2016-10-05 13:47:20]

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

[commit-bot: I haz the power, 2016-10-05 13:47:31]

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

[mvstanton, 2016-10-05 13:50:09]

Description was changed from

==========
[Turbofan] Introduce OtherNumberConstant.

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

to

==========
[Turbofan] Introduce OtherNumberConstant.

With this CL, we devolve all Constants introduced as they are with an object
handle into

* Range - for integers
* Nan
* MinusZero
* OtherNumberConstant - for doubles
* HeapConstant

We reduce the amount we have to inspect an object handle during optimization.
Also, simplifications result. For example, you never have to check if a Range
contains a HeapConstant. 

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

[commit-bot: I haz the power, 2016-10-05 13:50:43]

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

[commit-bot: I haz the power, 2016-10-05 13:50:44]

Dry run: Try jobs failed on following builders:
  v8_presubmit on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/25681)

[mvstanton, 2016-10-05 13:51:10]

Hi Jaro,
Comment response. I was able to remove code that compares Ranges and
HeapConstants together.
Thanks for the look,
--Mike

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/typer.cc
File src/compiler/typer.cc (right):

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/typer.cc#n...
src/compiler/typer.cc:595: return Type::NewConstant(f->NewNumber(number),
zone());
On 2016/10/04 13:14:55, Jarin wrote:
> How about having a factory from a double value?

Done.

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/types.cc
File src/compiler/types.cc (right):

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/types.cc#n...
src/compiler/types.cc:85: if (this->IsConstant()) return
this->AsConstant()->Value()->Number();
On 2016/10/04 13:14:55, Jarin wrote:
> Remove this line, please (Constants should be only heap objects now).

Done.

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/types.cc#n...
src/compiler/types.cc:103: if (this->IsConstant()) return
this->AsConstant()->Value()->Number();
On 2016/10/04 13:14:55, Jarin wrote:
> Remove.

Done.

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/types.h
File src/compiler/types.h (right):

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/types.h#ne...
src/compiler/types.h:301: return !std::isnan(value) && !IsInteger(value) &&
!i::IsMinusZero(value);
On 2016/10/04 13:14:55, Jarin wrote:
> IsInteger => Type::IsInteger (and remove the IsInteger method above?)

Let me keep this for now, I wouldn't be able to inline very well because of
definition ordering.

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/types.h#ne...
src/compiler/types.h:306: return
IsOtherNumberConstant(HeapNumber::cast(value)->value());
On 2016/10/04 13:14:55, Jarin wrote:
> Nit: 
> 
> return value->IsHeapNumber() &&
>     IsOtherNumberConstant(HeapNumber::cast(value)->value());

Done.

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/types.h#ne...
src/compiler/types.h:335: class ConstantType : public TypeBase {
On 2016/10/04 13:14:55, Jarin wrote:
> Maybe ConstantType => HeapConstantType?

Done.

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/types.h#ne...
src/compiler/types.h:527: static Type* Constant(i::Handle<i::Object> value,
Zone* zone) {
On 2016/10/04 13:14:55, Jarin wrote:
> Maybe Constant => HeapConstant?

Done.

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/types.h#ne...
src/compiler/types.h:571: bool IsConstant() { return
IsKind(TypeBase::kConstant); }
On 2016/10/04 13:14:55, Jarin wrote:
> IsHeapConstant?

Done.

[mvstanton, 2016-10-05 13:58:13]

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

[commit-bot: I haz the power, 2016-10-05 13:58:22]

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

[mvstanton, 2016-10-05 14:02:55]

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

[commit-bot: I haz the power, 2016-10-05 14:03:06]

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

[Jarin, 2016-10-05 14:06:53]

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/typer.cc
File src/compiler/typer.cc (right):

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/typer.cc#n...
src/compiler/typer.cc:595: return Type::NewConstant(f->NewNumber(number),
zone());
On 2016/10/05 13:51:10, mvstanton wrote:
> On 2016/10/04 13:14:55, Jarin wrote:
> > How about having a factory from a double value?
> 
> Done.

How about using it, then?

[commit-bot: I haz the power, 2016-10-05 14:07:17]

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

[commit-bot: I haz the power, 2016-10-05 14:07:18]

Dry run: Try jobs failed on following builders:
  v8_presubmit on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/25685)

[mvstanton, 2016-10-05 14:09:44]

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

[commit-bot: I haz the power, 2016-10-05 14:09:46]

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

[mvstanton, 2016-10-05 14:14:44]

Thanks for your comments.
Have another look, thanks.
--Mike

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/typer.cc
File src/compiler/typer.cc (right):

https://codereview.chromium.org/2381523002/diff/60001/src/compiler/typer.cc#n...
src/compiler/typer.cc:595: return Type::NewConstant(f->NewNumber(number),
zone());
On 2016/10/05 14:06:53, Jarin wrote:
> On 2016/10/05 13:51:10, mvstanton wrote:
> > On 2016/10/04 13:14:55, Jarin wrote:
> > > How about having a factory from a double value?
> > 
> > Done.
> 
> How about using it, then?

My apologies. This time I have used it.

[mvstanton, 2016-10-05 14:51:17]

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

[commit-bot: I haz the power, 2016-10-05 14:51:26]

Dry run: 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-10-05 14:57:16]

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

[commit-bot: I haz the power, 2016-10-05 14:57:18]

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

[mvstanton, 2016-10-06 08:08:21]

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

[commit-bot: I haz the power, 2016-10-06 08:08:31]

Dry run: 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-10-06 08:44:32]

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

[commit-bot: I haz the power, 2016-10-06 08:44:33]

Dry run: This issue passed the CQ dry run.

[mvstanton, 2016-10-06 13:24:36]

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

[commit-bot: I haz the power, 2016-10-06 13:24:41]

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

[mvstanton, 2016-10-06 13:26:08]

Another update, thanks, should be finally done. :)
--Mike

[Jarin, 2016-10-06 13:41:15]

lgtm, thanks!

[commit-bot: I haz the power, 2016-10-06 13:55:35]

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

[commit-bot: I haz the power, 2016-10-06 13:55:36]

Dry run: This issue passed the CQ dry run.

[mvstanton, 2016-10-06 14:02:26]

The CQ bit was checked by mvstanton@chromium.org

[commit-bot: I haz the power, 2016-10-06 14:02:35]

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-10-06 14:13:37]

Description was changed from

==========
[Turbofan] Introduce OtherNumberConstant.

With this CL, we devolve all Constants introduced as they are with an object
handle into

* Range - for integers
* Nan
* MinusZero
* OtherNumberConstant - for doubles
* HeapConstant

We reduce the amount we have to inspect an object handle during optimization.
Also, simplifications result. For example, you never have to check if a Range
contains a HeapConstant. 

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

to

==========
[Turbofan] Introduce OtherNumberConstant.

With this CL, we devolve all Constants introduced as they are with an object
handle into

* Range - for integers
* Nan
* MinusZero
* OtherNumberConstant - for doubles
* HeapConstant

We reduce the amount we have to inspect an object handle during optimization.
Also, simplifications result. For example, you never have to check if a Range
contains a HeapConstant. 

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

[commit-bot: I haz the power, 2016-10-06 14:13:38]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2016-10-06 14:13:58]

Description was changed from

==========
[Turbofan] Introduce OtherNumberConstant.

With this CL, we devolve all Constants introduced as they are with an object
handle into

* Range - for integers
* Nan
* MinusZero
* OtherNumberConstant - for doubles
* HeapConstant

We reduce the amount we have to inspect an object handle during optimization.
Also, simplifications result. For example, you never have to check if a Range
contains a HeapConstant. 

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

to

==========
[Turbofan] Introduce OtherNumberConstant.

With this CL, we devolve all Constants introduced as they are with an object
handle into

* Range - for integers
* Nan
* MinusZero
* OtherNumberConstant - for doubles
* HeapConstant

We reduce the amount we have to inspect an object handle during optimization.
Also, simplifications result. For example, you never have to check if a Range
contains a HeapConstant.

BUG=

Committed: https://crrev.com/978fe70beb3d297e34037d3776b39e9018d52980
Cr-Commit-Position: refs/heads/master@{#40041}
==========

[commit-bot: I haz the power, 2016-10-06 14:13:59]

Patchset 12 (id:??) landed as
https://crrev.com/978fe70beb3d297e34037d3776b39e9018d52980
Cr-Commit-Position: refs/heads/master@{#40041}