Add UInt32 and UInt31 types to better infer bit operations.

Add UInt32 and UInt31 types to better infer bit operations.

R=kasperl@google.com

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

[kasperl, 2013-11-26 14:58:40]

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
File
sdk/lib/_internal/compiler/implementation/inferrer/simple_types_inferrer.dart
(right):

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/inferrer/simple_types_inferrer.dart:77:
TypeMask get uint32Type => compiler.typesTask.uint32Type;
Add uint31Type here?

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/inferrer/type_graph_inferrer.dart
(right):

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_inferrer.dart:510:
var value =
What's the type of this?

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
File
sdk/lib/_internal/compiler/implementation/ssa/invoke_dynamic_specializers.dart
(right):

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/ssa/invoke_dynamic_specializers.dart:395:
if (left.isUInt31(compiler) && right.isUInt31(compiler)) {
Isn't it enough if right is positive?

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/ssa/nodes.dart (right):

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/ssa/nodes.dart:906: return
instructionType.satisfies(backend.jsUInt32Class, compiler)
Maybe do !isNullable first -- that's probably a little bit cheaper.

[floitsch, 2013-11-26 15:08:19]

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart
(right):

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart:530:
bool intCheck(info) => info.type.containsOnlyInt(compiler);
I don't like the name "check". "predicate" or similar would fit better imho.

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart:547: &&
arguments.hasOnePositionalArgumentThatMatches(uint31Check)) {
">>" doesn't need the right-hand side.
If uint31 >> x succeeds it will be a uint31, independent of x. If you need to
make sure that the operation succeeds you can check for uint32.

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/ssa/codegen.dart (right):

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/ssa/codegen.dart:209: // integer.
Also, if we are using & with a positive constant we know
Update comment.

[ngeoffray, 2013-11-26 15:19:12]

Thanks Kasper and Florian. PTAL.

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
File
sdk/lib/_internal/compiler/implementation/inferrer/simple_types_inferrer.dart
(right):

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/inferrer/simple_types_inferrer.dart:77:
TypeMask get uint32Type => compiler.typesTask.uint32Type;
On 2013/11/26 14:58:40, kasperl wrote:
> Add uint31Type here?

Done.

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/inferrer/type_graph_inferrer.dart
(right):

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_inferrer.dart:510:
var value =
On 2013/11/26 14:58:40, kasperl wrote:
> What's the type of this?

Constant. I need to add a 'show' to type it. Done.

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart
(right):

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart:530:
bool intCheck(info) => info.type.containsOnlyInt(compiler);
On 2013/11/26 15:08:20, floitsch wrote:
> I don't like the name "check". "predicate" or similar would fit better imho.

Used is* instead.

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart:547: &&
arguments.hasOnePositionalArgumentThatMatches(uint31Check)) {
On 2013/11/26 15:08:20, floitsch wrote:
> ">>" doesn't need the right-hand side.
> If uint31 >> x succeeds it will be a uint31, independent of x. If you need to
> make sure that the operation succeeds you can check for uint32.

Done.

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/ssa/codegen.dart (right):

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/ssa/codegen.dart:209: // integer.
Also, if we are using & with a positive constant we know
On 2013/11/26 15:08:20, floitsch wrote:
> Update comment.

Done.

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
File
sdk/lib/_internal/compiler/implementation/ssa/invoke_dynamic_specializers.dart
(right):

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/ssa/invoke_dynamic_specializers.dart:395:
if (left.isUInt31(compiler) && right.isUInt31(compiler)) {
On 2013/11/26 14:58:40, kasperl wrote:
> Isn't it enough if right is positive?

You mean left, right? :-) Done.

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/ssa/nodes.dart (right):

https://codereview.chromium.org/87783003/diff/70001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/ssa/nodes.dart:906: return
instructionType.satisfies(backend.jsUInt32Class, compiler)
On 2013/11/26 14:58:40, kasperl wrote:
> Maybe do !isNullable first -- that's probably a little bit cheaper.

Done.

[kasperl, 2013-11-26 18:53:09]

LGTM.

https://codereview.chromium.org/87783003/diff/90001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart
(right):

https://codereview.chromium.org/87783003/diff/90001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart:546: if
(isUInt31(receiver)
So I guess it's important to realize that the fallback (if this doesn't match)
is to get the inferred type for the operation which is quite possibly Uint32,
right? Maybe that should be written as a comment somewhere in this method.

[ngeoffray, 2013-11-27 07:39:52]

Thanks Kasper.

https://codereview.chromium.org/87783003/diff/90001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart
(right):

https://codereview.chromium.org/87783003/diff/90001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/inferrer/type_graph_nodes.dart:546: if
(isUInt31(receiver)
On 2013/11/26 18:53:09, kasperl wrote:
> So I guess it's important to realize that the fallback (if this doesn't match)
> is to get the inferred type for the operation which is quite possibly Uint32,
> right? Maybe that should be written as a comment somewhere in this method.

Done.

[ngeoffray, 2013-11-27 07:42:13]

Committed patchset #3 manually as r30693 (presubmit successful).