dart2js cps: Type casts and related changes to type propagation.

dart2js cps: Type casts and related changes to type propagation.

We now optimize type casts that always fail or always pass.
Type propagation has been refactored a bit to make this work.

A new IR node Unreachable has been added. This is an node that is known
to be unreachable and is ultimately compiled to an empty statement.

BUG=
R=karlklose@google.com

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

[asgerf, 2015-06-04 09:35:13]

asgerf@google.com changed reviewers:
+ karlklose@google.com, kmillikin@google.com, sra@google.com

[karlklose, 2015-06-08 09:46:53]

LGTM.

https://codereview.chromium.org/1153603006/diff/20001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart (right):

https://codereview.chromium.org/1153603006/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart:367: /// Therefore, [type] should
not be [Object], `dynamic`, or [Null], since `null`
'should' -> 'can't'?

https://codereview.chromium.org/1153603006/diff/20001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/type_propagation.dart (right):

https://codereview.chromium.org/1153603006/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:529: cont.body = new
Unreachable()..parent = cont;
How about making parent a constructor parameter?

https://codereview.chromium.org/1153603006/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:619: /// and without any side
effects.
Perhaps add: 'These expressions can be replaced by the constant they evaluate
to.'

https://codereview.chromium.org/1153603006/diff/20001/tests/language/language...
File tests/language/language_dart2js.status (right):

https://codereview.chromium.org/1153603006/diff/20001/tests/language/language...
tests/language/language_dart2js.status:955: mixin_mixin7_test : RuntimeError
Remove space before ':' and sort the line into the rest of the block.

Could you add why it fails?

[asgerf, 2015-06-08 10:46:17]

https://codereview.chromium.org/1153603006/diff/20001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart (right):

https://codereview.chromium.org/1153603006/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart:367: /// Therefore, [type] should
not be [Object], `dynamic`, or [Null], since `null`
On 2015/06/08 09:46:53, karlklose wrote:
> 'should' -> 'can't'?

Hmm.. the comment is actually outdated and wrong. I forgot to update it after I
found out that it wasn't true, because [type] can be a type variable containing
one of those types at runtime (and it is even quite common for a type variable
to contain dynamic).

I've updated with a new one.

https://codereview.chromium.org/1153603006/diff/20001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/type_propagation.dart (right):

https://codereview.chromium.org/1153603006/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:529: cont.body = new
Unreachable()..parent = cont;
On 2015/06/08 09:46:53, karlklose wrote:
> How about making parent a constructor parameter?

For all CPS nodes? That would be great burden on the CPS builder. I guess it
could be an optional parameter, but to me it seems odd.

More generally, here is how I think of it: things that affect the operational
semantics of the IR go in the constructor. Auxiliary information like parent
pointers, isReceiverNotNull, etc, are just fields that are initialized
elsewhere.

https://codereview.chromium.org/1153603006/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:619: /// and without any side
effects.
On 2015/06/08 09:46:53, karlklose wrote:
> Perhaps add: 'These expressions can be replaced by the constant they evaluate
> to.'

Right, the comment did not really explain why the field is called
'replacements'.

https://codereview.chromium.org/1153603006/diff/20001/tests/language/language...
File tests/language/language_dart2js.status (right):

https://codereview.chromium.org/1153603006/diff/20001/tests/language/language...
tests/language/language_dart2js.status:955: mixin_mixin7_test : RuntimeError
On 2015/06/08 09:46:53, karlklose wrote:
> Remove space before ':' and sort the line into the rest of the block.
> 
> Could you add why it fails?

This test also fails in SSA. For a short while it passed for the wrong reasons
in CPS, but now it fails for the same reason as SSA. I've moved it back to its
original place.

As far as I can tell, the RTI system looks for things like "is" checks on type
variables to determine if an $asT function should be emitted for a given
subclass of T, but does not check for type variables of T being reified, which
also depends on $asT.

[karlklose, 2015-06-08 11:00:27]

Still LGTM.

[asgerf, 2015-06-08 15:02:54]

Committed patchset #6 (id:90001) manually as
d01d394af7f3e12c9577490820820330e216bece (presubmit successful).