Fix usage of UNKNOWN/CONFLICTING types for union.

Fix usage of UNKNOWN/CONFLICTING types for union.

Fixes issue 4033.

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

[floitsch, 2012-07-11 10:12:46]

known to tank performance.

https://chromiumcodereview.appspot.com/10735051/diff/2001/lib/compiler/implem...
File lib/compiler/implementation/compiler.dart (right):

https://chromiumcodereview.appspot.com/10735051/diff/2001/lib/compiler/implem...
lib/compiler/implementation/compiler.dart:172: return HType.UNKNOWN;
I didn't change this UNKNOWN into a CONFLICTING. I think it is right now.

[Mads Ager (google), 2012-07-11 10:24:34]

LGTM!

Good to have this fixed so we can actually get *correct* information from using
the union operation on types. I'll attempt to restore some of the field type
optimizations in a safe way.

https://chromiumcodereview.appspot.com/10735051/diff/5001/lib/compiler/implem...
File lib/compiler/implementation/ssa/optimize.dart (right):

https://chromiumcodereview.appspot.com/10735051/diff/5001/lib/compiler/implem...
lib/compiler/implementation/ssa/optimize.dart:1178: if (!type.isConflicting() &&
!type.isUnknown()) {
I think we can relax this check but I will look into that once you submit this.

[sra1, 2012-07-11 20:04:33]

I think representing various points in the type lattice by a rich hierarchy of
classes is the wrong way to go.
Operations will be less error-prone if the type space is constructed as a union
of disjoint types, e.g. as a bit mask.  The operations can then be coded once
rather than having niggling bugs and asymmetry in the operations.

https://chromiumcodereview.appspot.com/10735051/diff/5001/lib/compiler/implem...
File lib/compiler/implementation/ssa/types.dart (right):

https://chromiumcodereview.appspot.com/10735051/diff/5001/lib/compiler/implem...
lib/compiler/implementation/ssa/types.dart:117: * A union of [CONFLICTING] with
any other types returns the other type.
I find CONFLICTING to be a confusing name.
The element is really the bottom element of a bounded lattice, which represents
the empty set of values.  For there to be a 'conflict' two things have to be in
opposition.  In a 0-1-many analysis, two is 'many', making it feel like
CONFLICTING is the top element, i.e. the same as UNKNOWN.
I suspect the bugs fixed in this change are due to that confusion.

I suggest renaming CONFLICTING to NOTHING, EMPTY, BOTTOM, NO_VALUES or something
similar.

If you choose BOTTOM, for consistency also change UNKNOWN to TOP.

https://chromiumcodereview.appspot.com/10735051/diff/5001/lib/compiler/implem...
lib/compiler/implementation/ssa/types.dart:165: if (other.isUnknown()) return
HType.UNKNOWN;
Every union method needs to accomplish the bounded lattice axioms

   this union CONFLICTING => this
   this union this => this
   this union UNKNOWN => UNKNOWN

Would it make sense for the base class to have a union method that does these
tests and calls _union only when the types don't match the axioms?

https://chromiumcodereview.appspot.com/10735051/diff/5001/lib/compiler/implem...
lib/compiler/implementation/ssa/types.dart:171: if (!other.canBeNull()) return
HType.UNKNOWN;
This is too pessimistic.  HBoundedType should become nullable.