cps-ir: Clean-ups in type propagator.

cps-ir: Clean-ups in type propagator.

R=karlklose@google.com, kmillikin@google.com

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

[asgerf, 2015-04-15 14:28:58]

asgerf@google.com changed reviewers:
+ karlklose@google.com

[asgerf, 2015-04-15 14:28:59]

I wanted to play with the type propagator and stumbled across some things that
seem wrong.

https://codereview.chromium.org/1088153002/diff/1/pkg/compiler/lib/src/cps_ir...
File pkg/compiler/lib/src/cps_ir/type_propagation.dart (left):

https://codereview.chromium.org/1088153002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:527: // a type-check (in
checked mode) are still executed.
Short-circuiting operators are not in the CPS.

https://codereview.chromium.org/1088153002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:811: if
(!typeSystem.areAssignable(leftType, rightType)) {
The check needed here is disjointness, but I don't think TypeMasks have an
efficient check for this.

https://codereview.chromium.org/1088153002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:901: return kind |
(constant.hashCode * 5) | type.hashCode * 7;
I'm not an expert on hash codes, but I don't think bitwise OR is a paricularly
good idea.

[Kevin Millikin (Google), 2015-04-15 14:51:51]

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

[Kevin Millikin (Google), 2015-04-15 14:51:51]

OK, LGTM.

https://codereview.chromium.org/1088153002/diff/1/pkg/compiler/lib/src/cps_ir...
File pkg/compiler/lib/src/cps_ir/type_propagation.dart (right):

https://codereview.chromium.org/1088153002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:311: _AbstractValue<T>
nonConst([T type]) {
This should probably be nonConstant.  It reads better at the cost of three
characters.

https://codereview.chromium.org/1088153002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:319: return new
_AbstractValue<T>.constValue(constant, type);
constantValue or constant please.

[asgerf, 2015-04-15 15:00:53]

https://codereview.chromium.org/1088153002/diff/1/pkg/compiler/lib/src/cps_ir...
File pkg/compiler/lib/src/cps_ir/type_propagation.dart (right):

https://codereview.chromium.org/1088153002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:319: return new
_AbstractValue<T>.constValue(constant, type);
On 2015/04/15 14:51:51, kmillikin wrote:
> constantValue or constant please.

`constant` is taken by field in the class, so constantValue it is.

[karlklose, 2015-04-16 06:32:08]

LGTM.

https://codereview.chromium.org/1088153002/diff/1/pkg/compiler/lib/src/cps_ir...
File pkg/compiler/lib/src/cps_ir/type_propagation.dart (right):

https://codereview.chromium.org/1088153002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:871: return kind * 31 +
constant.hashCode * 157 + type.hashCode * 1721;
I think smaller primes are sufficient, since you already work on hash 
codes.  Maybe add a constant factor.  Also make sure that the result stays in 30
bit range.

[asgerf, 2015-04-16 07:51:17]

https://codereview.chromium.org/1088153002/diff/1/pkg/compiler/lib/src/cps_ir...
File pkg/compiler/lib/src/cps_ir/type_propagation.dart (right):

https://codereview.chromium.org/1088153002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:871: return kind * 31 +
constant.hashCode * 157 + type.hashCode * 1721;
On 2015/04/16 06:32:08, karlklose wrote:
> I think smaller primes are sufficient, since you already work on hash 
> codes.  Maybe add a constant factor.  Also make sure that the result stays in
30
> bit range.

Done.

[asgerf, 2015-04-16 08:12:11]

Committed patchset #5 (id:80001) manually as 45195 (presubmit successful).