Add bool value types to type inference.

dart2js: add bool value types

Also optimize if-statements which have a condition with an inferred value

BUG=
R=sigmund@google.com

Committed: https://github.com/dart-lang/sdk/commit/0132043db489ff81d67396ec1dcb248ef6ba3068

[Harry Terkelsen, 2015-09-28 22:05:39]

het@google.com changed reviewers:
+ sigmund@google.com, sra@google.com

[Harry Terkelsen, 2015-09-28 22:05:40]

ng2 hello_world codesize before: 1526085
ng2 hello_world codesize after:  1426238
reduction of 7%

[Siggi Cherem (dart-lang), 2015-09-28 22:13:53]

very cool!

Could you add a test (like the type_inference2_test.dart) that would show that
this is working as you intended?

[Harry Terkelsen, 2015-09-28 22:58:20]

ptal, added test

[herhut, 2015-09-28 23:55:29]

herhut@google.com changed reviewers:
+ herhut@google.com

[herhut, 2015-09-28 23:55:30]

Nice!

https://codereview.chromium.org/1371193004/diff/40001/tests/compiler/dart2js/...
File tests/compiler/dart2js/type_inference8_test.dart (right):

https://codereview.chromium.org/1371193004/diff/40001/tests/compiler/dart2js/...
tests/compiler/dart2js/type_inference8_test.dart:13: const String TEST = r"""
Can you also add a test where it is supposed to fail and infer just bool?

[Siggi Cherem (dart-lang), 2015-09-29 00:08:33]

thanks Harry!

lgtm with the comments below

https://codereview.chromium.org/1371193004/diff/40001/tests/compiler/dart2js/...
File tests/compiler/dart2js/type_inference8_test.dart (right):

https://codereview.chromium.org/1371193004/diff/40001/tests/compiler/dart2js/...
tests/compiler/dart2js/type_inference8_test.dart:13: const String TEST = r"""
On 2015/09/28 23:55:30, herhut wrote:
> Can you also add a test where it is supposed to fail and infer just bool?

+1 :)

https://codereview.chromium.org/1371193004/diff/40001/tests/compiler/dart2js/...
tests/compiler/dart2js/type_inference8_test.dart:14: foo(x) {
would this work if we make foo non-inlinable?

[Harry Terkelsen, 2015-09-29 00:42:54]

https://codereview.chromium.org/1371193004/diff/40001/tests/compiler/dart2js/...
File tests/compiler/dart2js/type_inference8_test.dart (right):

https://codereview.chromium.org/1371193004/diff/40001/tests/compiler/dart2js/...
tests/compiler/dart2js/type_inference8_test.dart:13: const String TEST = r"""
On 2015/09/28 23:55:30, herhut wrote:
> Can you also add a test where it is supposed to fail and infer just bool?

Done.

https://codereview.chromium.org/1371193004/diff/40001/tests/compiler/dart2js/...
tests/compiler/dart2js/type_inference8_test.dart:14: foo(x) {
On 2015/09/29 00:08:33, Siggi Cherem (dart-lang) wrote:
> would this work if we make foo non-inlinable?

the compiler returned by compilerFor from compiler_helper.dart has disabled
inlining

[Harry Terkelsen, 2015-09-29 00:44:30]

Committed patchset #4 (id:60001) manually as
0132043db489ff81d67396ec1dcb248ef6ba3068 (presubmit successful).

[sra1, 2015-09-29 01:02:05]

https://codereview.chromium.org/1371193004/diff/40001/pkg/compiler/lib/src/in...
File pkg/compiler/lib/src/inferrer/type_graph_inferrer.dart (right):

https://codereview.chromium.org/1371193004/diff/40001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/inferrer/type_graph_inferrer.dart:247: return new
BoolLiteralTypeInformation(value, compiler.typesTask.boolType);
Is there a way to start tracking from a non-literal?
E.g.


    foo(const bool.fromEnvironment("DEBUG") == true);

https://codereview.chromium.org/1371193004/diff/40001/pkg/compiler/lib/src/ss...
File pkg/compiler/lib/src/ssa/optimize.dart (right):

https://codereview.chromium.org/1371193004/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/optimize.dart:1310: ValueTypeMask valueType =
condition.instructionType;
I don't think we should it like this.

We should rather replace values that are inferred to a constant bool with a
HConstant in SsaInstructionSimplifier.  That will (1) sever dependencies and (2)
enable constant folding.

E.g.

    r = massiveFnReturningArgument(false);
    if (r != null)
      print(r);

becomes

    massiveFnReturningArgument(false);
    P.printLine("false");


1. Add helper method to SsaInstructionSimplifier to check and replace all uses
of an instruction with the appropriate HConstant if appropriate.
2. Add SsaInstructionSimplifier.visitParameterValue and call the helper to
convert values coming in from call site arguments.
3. Add calls at a few of the visitInvokeXXX methods too convert values returned
from call sites.

[Harry Terkelsen, 2015-09-29 01:09:53]

Thanks Stephen

https://codereview.chromium.org/1371193004/diff/40001/pkg/compiler/lib/src/in...
File pkg/compiler/lib/src/inferrer/type_graph_inferrer.dart (right):

https://codereview.chromium.org/1371193004/diff/40001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/inferrer/type_graph_inferrer.dart:247: return new
BoolLiteralTypeInformation(value, compiler.typesTask.boolType);
On 2015/09/29 01:02:04, sra1 wrote:
> Is there a way to start tracking from a non-literal?
> E.g.
> 
> 
>     foo(const bool.fromEnvironment("DEBUG") == true);

Not yet, I want to add support for const bool expressions. Will add in upcoming
CL.

https://codereview.chromium.org/1371193004/diff/40001/pkg/compiler/lib/src/ss...
File pkg/compiler/lib/src/ssa/optimize.dart (right):

https://codereview.chromium.org/1371193004/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/optimize.dart:1310: ValueTypeMask valueType =
condition.instructionType;
On 2015/09/29 01:02:05, sra1 wrote:
> I don't think we should it like this.
> 
> We should rather replace values that are inferred to a constant bool with a
> HConstant in SsaInstructionSimplifier.  That will (1) sever dependencies and
(2)
> enable constant folding.
> 
> E.g.
> 
>     r = massiveFnReturningArgument(false);
>     if (r != null)
>       print(r);
> 
> becomes
> 
>     massiveFnReturningArgument(false);
>     P.printLine("false");
> 
> 
> 1. Add helper method to SsaInstructionSimplifier to check and replace all uses
> of an instruction with the appropriate HConstant if appropriate.
> 2. Add SsaInstructionSimplifier.visitParameterValue and call the helper to
> convert values coming in from call site arguments.
> 3. Add calls at a few of the visitInvokeXXX methods too convert values
returned
> from call sites.

I wanted to do this but I didn't know how to convert to an HConstant while also
doing calls for the effects. What is the correct way?

[Harry Terkelsen, 2015-09-29 01:16:34]

https://codereview.chromium.org/1371193004/diff/40001/pkg/compiler/lib/src/ss...
File pkg/compiler/lib/src/ssa/optimize.dart (right):

https://codereview.chromium.org/1371193004/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/optimize.dart:1310: ValueTypeMask valueType =
condition.instructionType;
On 2015/09/29 01:09:53, Harry Terkelsen wrote:
> On 2015/09/29 01:02:05, sra1 wrote:
> > I don't think we should it like this.
> > 
> > We should rather replace values that are inferred to a constant bool with a
> > HConstant in SsaInstructionSimplifier.  That will (1) sever dependencies and
> (2)
> > enable constant folding.
> > 
> > E.g.
> > 
> >     r = massiveFnReturningArgument(false);
> >     if (r != null)
> >       print(r);
> > 
> > becomes
> > 
> >     massiveFnReturningArgument(false);
> >     P.printLine("false");
> > 
> > 
> > 1. Add helper method to SsaInstructionSimplifier to check and replace all
uses
> > of an instruction with the appropriate HConstant if appropriate.
> > 2. Add SsaInstructionSimplifier.visitParameterValue and call the helper to
> > convert values coming in from call site arguments.
> > 3. Add calls at a few of the visitInvokeXXX methods too convert values
> returned
> > from call sites.
> 
> I wanted to do this but I didn't know how to convert to an HConstant while
also
> doing calls for the effects. What is the correct way?

Oops, replied before fully reading your comment. I'll implement this

[sra1, 2015-09-29 01:20:26]

On 2015/09/29 01:16:34, Harry Terkelsen wrote:
>
https://codereview.chromium.org/1371193004/diff/40001/pkg/compiler/lib/src/ss...
> File pkg/compiler/lib/src/ssa/optimize.dart (right):
> 
>
https://codereview.chromium.org/1371193004/diff/40001/pkg/compiler/lib/src/ss...
> pkg/compiler/lib/src/ssa/optimize.dart:1310: ValueTypeMask valueType =
> condition.instructionType;
> On 2015/09/29 01:09:53, Harry Terkelsen wrote:
> > On 2015/09/29 01:02:05, sra1 wrote:
> > > I don't think we should it like this.
> > > 
> > > We should rather replace values that are inferred to a constant bool with
a
> > > HConstant in SsaInstructionSimplifier.  That will (1) sever dependencies
and
> > (2)
> > > enable constant folding.
> > > 
> > > E.g.
> > > 
> > >     r = massiveFnReturningArgument(false);
> > >     if (r != null)
> > >       print(r);
> > > 
> > > becomes
> > > 
> > >     massiveFnReturningArgument(false);
> > >     P.printLine("false");
> > > 
> > > 
> > > 1. Add helper method to SsaInstructionSimplifier to check and replace all
> uses
> > > of an instruction with the appropriate HConstant if appropriate.
> > > 2. Add SsaInstructionSimplifier.visitParameterValue and call the helper to
> > > convert values coming in from call site arguments.
> > > 3. Add calls at a few of the visitInvokeXXX methods too convert values
> > returned
> > > from call sites.
> > 
> > I wanted to do this but I didn't know how to convert to an HConstant while
> also
> > doing calls for the effects. What is the correct way?
> 
> Oops, replied before fully reading your comment. I'll implement this

Thanks.

Take a look at the before/after diff on swarm. There is some pretty weird
looking code because other optimizations don't know the condition is decided. 
Converting to a constant will fix all of those.

[sra1, 2015-09-29 04:12:17]

DBC

https://codereview.chromium.org/1371193004/diff/60001/pkg/compiler/lib/src/in...
File pkg/compiler/lib/src/inferrer/type_graph_nodes.dart (right):

https://codereview.chromium.org/1371193004/diff/60001/pkg/compiler/lib/src/in...
pkg/compiler/lib/src/inferrer/type_graph_nodes.dart:1133:
BoolLiteralTypeInformation(value, TypeMask mask)
What is the type of 'value'?

Why do we need this TypeInformation?
Should is not be sufficient to propagate the ValueTypeMask?