Add another type JSPositiveInt to show a range analysis in the inferrer would be very beneficial :-…

Add another type JSPositiveInt to show a range analysis in the inferrer would be very beneficial :-).

R=kasperl@google.com

Committed: https://code.google.com/p/dart/source/detail?r=30759

[kasperl, 2013-11-28 09:09:46]

LGTM, but consider changing positive -> non-negative in another CL.

https://codereview.chromium.org/94303002/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/constants.dart (right):

https://codereview.chromium.org/94303002/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/constants.dart:167: bool isPositive()
=> value >= 0;
Stephen would probably insist on this being isNonNegative. He's probably right
:-)

https://codereview.chromium.org/94303002/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart
(right):

https://codereview.chromium.org/94303002/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart:572:
return inferrer.types.intType;
Can't you do -0.1? Add a comment on why this is always an int.

https://codereview.chromium.org/94303002/diff/1/sdk/lib/_internal/compiler/im...
File
sdk/lib/_internal/compiler/implementation/ssa/invoke_dynamic_specializers.dart
(right):

https://codereview.chromium.org/94303002/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/invoke_dynamic_specializers.dart:229:
TypeMask computeTypeFromInputTypes(HInvokeDynamic instruction,
Share this code between add, modulo, multiply, truncating divide.

https://codereview.chromium.org/94303002/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/types/flat_type_mask.dart
(right):

https://codereview.chromium.org/94303002/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/types/flat_type_mask.dart:103: || cls
== backend.positiveIntImplementation;
Move up to have the same order as in the method below?

[sra1, 2013-11-28 09:21:03]

I think we can do better without this type :-)

JavaScript arrays and typed arrays have a UInt32 length.

I tried changing the 'length' and operator[] types here:
https://codereview.chromium.org/92713003/.

It made 2% difference on Sudoku by omitting <0 tests on indexes coming from
Uint16List, but there was a code quality regression somewhere else because I did
not change anything in the optimizer or codegen to match.  I was not sure that
2% was more than noise.

So consider using JSUInt32 rather than JSPositiveInt.

I think JSPositiveInt is still useful.
JSUInt32 + JSUInt32  => JSPositiveInt.
How do you propose to add the more refined transfer functions? 

Also, can we avoid calling it 'positive', since that traditionally excludes
zero.

https://codereview.chromium.org/94303002/diff/1/sdk/lib/_internal/lib/js_numb...
File sdk/lib/_internal/lib/js_number.dart (right):

https://codereview.chromium.org/94303002/diff/1/sdk/lib/_internal/lib/js_numb...
sdk/lib/_internal/lib/js_number.dart:369: class JSPositiveInt extends JSInt {}
Zero is not positive.
Nonnegative is a more accurate name.

[ngeoffray, 2013-11-28 11:21:47]

Thanks Kasper and Stephen.

I will rename the type in another CL.

https://codereview.chromium.org/94303002/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart
(right):

https://codereview.chromium.org/94303002/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart:572:
return inferrer.types.intType;
On 2013/11/28 09:09:47, kasperl wrote:
> Can't you do -0.1? Add a comment on why this is always an int.

There's a test at the beginning of this function whether the receiver is an int.

https://codereview.chromium.org/94303002/diff/1/sdk/lib/_internal/compiler/im...
File
sdk/lib/_internal/compiler/implementation/ssa/invoke_dynamic_specializers.dart
(right):

https://codereview.chromium.org/94303002/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/invoke_dynamic_specializers.dart:229:
TypeMask computeTypeFromInputTypes(HInvokeDynamic instruction,
On 2013/11/28 09:09:47, kasperl wrote:
> Share this code between add, modulo, multiply, truncating divide.

Done.

https://codereview.chromium.org/94303002/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/types/flat_type_mask.dart
(right):

https://codereview.chromium.org/94303002/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/types/flat_type_mask.dart:103: || cls
== backend.positiveIntImplementation;
On 2013/11/28 09:09:47, kasperl wrote:
> Move up to have the same order as in the method below?

Done.

[ngeoffray, 2013-11-28 11:22:52]

Committed patchset #2 manually as r30759 (presubmit successful).