Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(6)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: lib/compiler/implementation/ssa/nodes.dart

Issue 10139012: Refactor types in ssa nodes. (Closed) Base URL: https://dart.googlecode.com/svn/branches/bleeding_edge/dart
Patch Set: Address comments and remove isIntegerOrDouble. Created 8 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « lib/compiler/implementation/ssa/codegen.dart ('k') | lib/compiler/implementation/ssa/optimize.dart » ('j') | lib/compiler/implementation/ssa/optimize.dart » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/compiler/implementation/ssa/nodes.dart
diff --git a/lib/compiler/implementation/ssa/nodes.dart b/lib/compiler/implementation/ssa/nodes.dart
index 19f794894f0250e7588c7940155c9b81fbd85197..8bd753a7a952e70cc40517ceeab9d01c6a41064d 100644
--- a/lib/compiler/implementation/ssa/nodes.dart
+++ b/lib/compiler/implementation/ssa/nodes.dart
@@ -667,32 +667,25 @@ class HBasicBlock extends HInstructionList implements Hashable {
class HType {
- final int flag;
- const HType(int this.flag);
-
- static final int FLAG_CONFLICTING = 0;
- static final int FLAG_UNKNOWN = 1;
- static final int FLAG_BOOLEAN = FLAG_UNKNOWN << 1;
- static final int FLAG_INTEGER = FLAG_BOOLEAN << 1;
- static final int FLAG_STRING = FLAG_INTEGER << 1;
- static final int FLAG_READABLE_ARRAY = FLAG_STRING << 1;
- // FLAG_WRITABLE_ARRAY implies FLAG_READABLE_ARRAY.
- static final int FLAG_WRITEABLE_ARRAY = FLAG_READABLE_ARRAY << 1;
- static final int FLAG_DOUBLE = FLAG_WRITEABLE_ARRAY << 1;
- static final int FLAG_NON_PRIMITIVE = FLAG_DOUBLE << 1;
-
- static final HType CONFLICTING = const HType(FLAG_CONFLICTING);
- static final HType UNKNOWN = const HType(FLAG_UNKNOWN);
- static final HType BOOLEAN = const HType(FLAG_BOOLEAN);
- static final HType STRING = const HType(FLAG_STRING);
- static final HType READABLE_ARRAY = const HType(FLAG_READABLE_ARRAY);
- static final HType MUTABLE_ARRAY =
- const HType(FLAG_READABLE_ARRAY | FLAG_WRITEABLE_ARRAY);
- static final HType INTEGER = const HType(FLAG_INTEGER);
- static final HType DOUBLE = const HType(FLAG_DOUBLE);
- static final HType STRING_OR_ARRAY =
- const HType(FLAG_STRING | FLAG_READABLE_ARRAY | FLAG_WRITEABLE_ARRAY);
- static final HType NUMBER = const HType(FLAG_DOUBLE | FLAG_INTEGER);
+ // We need a field so that the different static types are not canonicalized.
ngeoffray 2012/04/23 10:19:21 types -> fields
floitsch 2012/04/24 15:25:18 Refactored.
+ // This way we don't need to do special work for the toString.
ngeoffray 2012/04/23 10:19:21 the toString -> [toString]
floitsch 2012/04/24 15:25:18 Refactored.
+ final String typeName;
+ const HType(this.typeName);
+
+ static final HType CONFLICTING = const HType("conflicting");
+ static final HType UNKNOWN = const HType("unknown");
+ static final HType BOOLEAN = const HType("boolean");
+ static final HType STRING = const HType("string");
+ static final HType READABLE_ARRAY = const HType("readable array");
+ // Note that mutable array is considered to be a subtype of readable array.
+ // That is, all values of type mutable array are also readable arrays, but not
+ // the inverse.
+ static final HType MUTABLE_ARRAY = const HType("mutable array");
+ static final HType INTEGER = const HType("integer");
+ static final HType DOUBLE = const HType("double");
+ static final HType INTEGER_OR_DOUBLE = const HType("integer or double");
+ // In this context "Array" means readable array.
ngeoffray 2012/04/23 10:19:21 I'd rather have the field named STRING_OR_READABLE
floitsch 2012/04/24 15:25:18 Refactored.
+ static final HType STRING_OR_ARRAY = const HType("string or array");
bool isConflicting() => this === CONFLICTING;
bool isUnknown() => this === UNKNOWN;
@@ -700,55 +693,99 @@ class HType {
bool isInteger() => this === INTEGER;
bool isDouble() => this === DOUBLE;
bool isString() => this === STRING;
- bool isArray() => (this.flag & FLAG_READABLE_ARRAY) != 0;
+ bool isReadableArray() => this === READABLE_ARRAY;
bool isMutableArray() => this === MUTABLE_ARRAY;
- bool isNumber() => (this.flag & (FLAG_INTEGER | FLAG_DOUBLE)) != 0;
- bool isStringOrArray() =>
- (this.flag & (FLAG_STRING | FLAG_READABLE_ARRAY)) != 0;
- bool isNonPrimitive() => this.flag === FLAG_NON_PRIMITIVE;
+ bool isStringOrArray() => this === STRING_OR_ARRAY;
+ bool isNonPrimitive() => false;
+ bool hasNoSubtypes() {
+ return isInteger() || isDouble() || isString() || isMutableArray();
+ }
+ bool isNumber() => isInteger() || isDouble() || this === INTEGER_OR_DOUBLE;
+ /** [isArray] does not include [isStringOrArray]. */
+ bool isArray() => isReadableArray() || isMutableArray();
+ bool isIndexablePrimitive() => isString() || isArray() || isStringOrArray();
/** A type is useful it is not unknown and not conflicting. */
- bool isUseful() => this !== UNKNOWN && this !== CONFLICTING;
-
- static HType getTypeFromFlag(int flag) {
- if (flag === CONFLICTING.flag) return CONFLICTING;
- if (flag === UNKNOWN.flag) return UNKNOWN;
- if (flag === BOOLEAN.flag) return BOOLEAN;
- if (flag === INTEGER.flag) return INTEGER;
- if (flag === DOUBLE.flag) return DOUBLE;
- if (flag === STRING.flag) return STRING;
- if (flag === READABLE_ARRAY.flag) return READABLE_ARRAY;
- if (flag === MUTABLE_ARRAY.flag) return MUTABLE_ARRAY;
- if (flag === NUMBER.flag) return NUMBER;
- if (flag === STRING_OR_ARRAY.flag) return STRING_OR_ARRAY;
- unreachable();
- }
+ bool isUseful() => !isUnknown() && !isConflicting();
- String toString() {
- if (isConflicting()) return 'conflicting';
- if (isUnknown()) return 'unknown';
- if (isBoolean()) return 'boolean';
- if (isInteger()) return 'integer';
- if (isDouble()) return 'double';
- if (isString()) return 'string';
- if (isMutableArray()) return 'mutable array';
- if (isArray()) return 'array';
- if (isNumber()) return 'number';
- if (isStringOrArray()) return 'string or array';
- unreachable();
+ String toString() => typeName;
+
+ /**
+ * The intersection of two types is the intersection of its values. For
+ * example:
+ * * INTEGER.intersect(INTEGER_OR_DOUBLE) => INTEGER.
+ * * DOUBLE.intersect(INTEGER) => CONFLICTING.
+ * * MUTABLE_ARRAY.intersect(READABLE_ARRAY) => MUTABLE_ARRAY.
+ *
+ * When there is no predefined type to represent the intersection returns
+ * [CONFLICTING].
+ */
+ HType intersect(HType other) {
ngeoffray 2012/04/23 10:19:21 intersect -> intersection
floitsch 2012/04/24 15:25:18 Done.
+ if (this === other) return this;
+ // Unknown has no influence on the type. Always just return the other type.
+ if (isUnknown()) return other;
+ if (other.isUnknown()) return this;
+
+ // Avoid testing some of the combinations by making the most concrete type
ngeoffray 2012/04/23 10:19:21 most concrete -> leaf type?
floitsch 2012/04/24 15:25:18 refactored.
+ // [:this:].
+ if (!hasNoSubtypes() && other.hasNoSubtypes()) {
ngeoffray 2012/04/23 10:19:21 double negation is annoying to read. You could add
floitsch 2012/04/24 15:25:18 refactored.
+ return other.intersect(this);
+ }
+
+ // If other is a super-type return [this].
ngeoffray 2012/04/23 10:19:21 [other]
ngeoffray 2012/04/23 10:19:21 super-type -> supertype of [this]
floitsch 2012/04/24 15:25:18 refactored.
floitsch 2012/04/24 15:25:18 refactored.
+ if (isInteger() && other === INTEGER_OR_DOUBLE) return this;
+ if (isDouble() && other === INTEGER_OR_DOUBLE) return this;
+ if (isString() && other.isStringOrArray()) return this;
+ if (isMutableArray() && other.isIndexablePrimitive()) return this;
+
+ // Handle the cases where neither side is a supertype of the other.
+ if (isArray() && other.isStringOrArray()) return this;
+ if (other.isArray() && isStringOrArray()) return other;
+
+ return CONFLICTING;
}
- HType combine(HType other) {
+ /**
+ * The union of two types is the union of its values. For example:
+ * * INTEGER.union(INTEGER_OR_DOUBLE) => INTEGER_OR_DOUBLE.
+ * * DOUBLE.union(INTEGER) => DOUBLE.
+ * * MUTABLE_ARRAY.union(READABLE_ARRAY) => READABLE_ARRAY.
+ *
+ * When there is no predefined type to represent the union returns
+ * [CONFLICTING].
+ */
+ HType union(HType other) {
+ if (this === other) return this;
+ // Unknown has no influence on the type. Always just return the other type.
if (isUnknown()) return other;
if (other.isUnknown()) return this;
- return getTypeFromFlag(this.flag & other.flag);
+
+ // Avoid testing some of the combinations by making the most concrete type
ngeoffray 2012/04/23 10:19:21 most concrete -> leaf type?
floitsch 2012/04/24 15:25:18 refactored.
+ // [:this:].
+ if (!hasNoSubtypes() && other.hasNoSubtypes()) {
+ return other.union(this);
+ }
+
+ // If [other] is a super type of [this] return [other];
ngeoffray 2012/04/23 10:19:21 super type -> supertype
floitsch 2012/04/24 15:25:18 refactored.
+ if (isNumber() && other === INTEGER_OR_DOUBLE) return other;
+ if (isArray() && other.isReadableArray()) return other;
+ if ((isString() || isArray()) && other.isStringOrArray()) return other;
+
+ // Handle the cases where neither side is a super type of the other.
ngeoffray 2012/04/23 10:19:21 super type -> supertype
floitsch 2012/04/24 15:25:18 refactored.
+ if (isNumber() && other.isNumber()) return INTEGER_OR_DOUBLE;
+ if (isString() && other.isArray()) return STRING_OR_ARRAY;
+ if (isArray() && other.isString()) return STRING_OR_ARRAY;
+
+ return CONFLICTING;
}
}
class HNonPrimitiveType extends HType {
final Type type;
- // TODO(ngeoffray): Add a HPrimitiveType to get rid of the flag.
- const HNonPrimitiveType(Type this.type) : super(HType.FLAG_NON_PRIMITIVE);
+ // TODO(ngeoffray): Add a HPrimitiveType to get rid of the string.
+ const HNonPrimitiveType(Type this.type) : super("non primitive");
+
+ bool isNonPrimitive() => true;
HType combine(HType other) {
if (other.isNonPrimitive()) {
@@ -759,6 +796,11 @@ class HNonPrimitiveType extends HType {
return HType.CONFLICTING;
}
+ // As long as we don't keep track of super/sub types for non-primitive types
+ // the intersection and union is the same.
+ HType intersect(HType other) => combine(other);
+ HType union(HType other) => combine(other);
+
String toString() => type.toString();
Element lookupMember(SourceString name) {
ClassElement classElement = type.element;
@@ -818,6 +860,7 @@ class HInstruction implements Hashable {
// All isFunctions work on the propagated types.
bool isArray() => propagatedType.isArray();
+ bool isReadableArray() => propagatedType.isReadableArray();
bool isMutableArray() => propagatedType.isMutableArray();
bool isBoolean() => propagatedType.isBoolean();
bool isInteger() => propagatedType.isInteger();
@@ -826,6 +869,7 @@ class HInstruction implements Hashable {
bool isString() => propagatedType.isString();
bool isTypeUnknown() => propagatedType.isUnknown();
bool isStringOrArray() => propagatedType.isStringOrArray();
+ bool isIndexablePrimitive() => propagatedType.isIndexablePrimitive();
bool isNonPrimitive() => propagatedType.isNonPrimitive();
/**
@@ -1203,8 +1247,7 @@ class HInvokeInterceptor extends HInvokeStatic {
}
bool isLengthGetterOnStringOrArray() {
- return isLengthGetter()
- && inputs[1].isStringOrArray();
+ return isLengthGetter() && inputs[1].isIndexablePrimitive();
}
String get builtinJsName() {
@@ -1237,7 +1280,7 @@ class HInvokeInterceptor extends HInvokeStatic {
// If the first argument is a string or an array and we invoke methods
// on it that mutate it, then we want to restrict the incoming type to be
// a mutable array.
- if (input == inputs[1] && input.isStringOrArray()) {
+ if (input == inputs[1] && input.isIndexablePrimitive()) {
if (name == const SourceString('add')
|| name == const SourceString('removeLast')) {
return HType.MUTABLE_ARRAY;
@@ -1299,7 +1342,8 @@ class HForeign extends HInstruction {
static HType computeTypeFromDeclaredType(DartString declaredType) {
if (declaredType.slowToString() == 'bool') return HType.BOOLEAN;
if (declaredType.slowToString() == 'int') return HType.INTEGER;
- if (declaredType.slowToString() == 'num') return HType.NUMBER;
+ if (declaredType.slowToString() == 'double') return HType.DOUBLE;
+ if (declaredType.slowToString() == 'num') return HType.INTEGER_OR_DOUBLE;
if (declaredType.slowToString() == 'String') return HType.STRING;
return HType.UNKNOWN;
}
@@ -1348,7 +1392,7 @@ class HBinaryArithmetic extends HInvokeBinary {
if (left.isInteger() && right.isInteger()) return left.propagatedType;
if (left.isNumber()) {
if (left.isDouble() || right.isDouble()) return HType.DOUBLE;
- return HType.NUMBER;
+ return HType.INTEGER_OR_DOUBLE;
}
return HType.UNKNOWN;
}
@@ -1361,7 +1405,7 @@ class HBinaryArithmetic extends HInvokeBinary {
// are numbers. If we don't know the outgoing type we try to make it a
// number.
if (propagatedType.isUnknown() || propagatedType.isNumber()) {
- return HType.NUMBER;
+ return HType.INTEGER_OR_DOUBLE;
}
// Even if the desired outgoing type is not a number we still want the
// second argument to be a number if the first one is a number. This will
@@ -1370,12 +1414,12 @@ class HBinaryArithmetic extends HInvokeBinary {
// TODO(floitsch): normally we shouldn't request a number, but simply
// throw an IllegalArgumentException if it isn't. This would be similar
// to the array case.
- if (input == right && left.isNumber()) return HType.NUMBER;
+ if (input == right && left.isNumber()) return HType.INTEGER_OR_DOUBLE;
return HType.UNKNOWN;
}
HType get likelyType() {
- if (left.isTypeUnknown()) return HType.NUMBER;
+ if (left.isTypeUnknown()) return HType.INTEGER_OR_DOUBLE;
return HType.UNKNOWN;
}
@@ -1398,7 +1442,7 @@ class HAdd extends HBinaryArithmetic {
if (left.isInteger() && right.isInteger()) return left.propagatedType;
if (left.isNumber()) {
if (left.isDouble() || right.isDouble()) return HType.DOUBLE;
- return HType.NUMBER;
+ return HType.INTEGER_OR_DOUBLE;
}
if (left.isString()) return HType.STRING;
return HType.UNKNOWN;
@@ -1418,14 +1462,14 @@ class HAdd extends HBinaryArithmetic {
// ask for a number. Note that we might return the input's (say 'left')
// type depending on its (the 'left's) type. But that shouldn't matter.
if (propagatedType.isNumber() || left.isNumber() || right.isNumber()) {
- return HType.NUMBER;
+ return HType.INTEGER_OR_DOUBLE;
}
return HType.UNKNOWN;
}
HType get likelyType() {
if (left.isString() || right.isString()) return HType.STRING;
- if (left.isTypeUnknown() || left.isNumber()) return HType.NUMBER;
+ if (left.isTypeUnknown() || left.isNumber()) return HType.INTEGER_OR_DOUBLE;
return HType.UNKNOWN;
}
@@ -1624,11 +1668,11 @@ class HInvokeUnary extends HInvokeStatic {
// want the outgoing type to be the input too.
// If we don't know the outgoing type we try to make it a number.
if (propagatedType.isNumber()) return propagatedType;
- if (propagatedType.isUnknown()) return HType.NUMBER;
+ if (propagatedType.isUnknown()) return HType.INTEGER_OR_DOUBLE;
return HType.UNKNOWN;
}
- HType get likelyType() => HType.NUMBER;
+ HType get likelyType() => HType.INTEGER_OR_DOUBLE;
abstract UnaryOperation get operation();
}
@@ -1849,7 +1893,7 @@ class HPhi extends HInstruction {
if (inputType.isUnknown()) {
seenUnknown = true;
} else {
- candidateType = candidateType.combine(inputType);
+ candidateType = candidateType.union(inputType);
ngeoffray 2012/04/23 10:19:21 Please add a simple comment or example on why this
floitsch 2012/04/24 15:25:18 Done.
if (candidateType.isConflicting()) return HType.CONFLICTING;
}
}
@@ -1881,7 +1925,7 @@ class HPhi extends HInstruction {
// Don't be too restrictive. If the agreed type is integer or double just
// say that the likely type is number. If more is expected the type will be
// propagated back.
- if (agreedType.isNumber()) return HType.NUMBER;
+ if (agreedType.isNumber()) return HType.INTEGER_OR_DOUBLE;
return agreedType;
}
@@ -1924,7 +1968,9 @@ class HRelational extends HInvokeBinary {
// only. With numbers the outgoing type is a boolean. If something else
// is desired, then numbers are incorrect, though.
if (propagatedType.isUnknown() || propagatedType.isBoolean()) {
- if (left.isTypeUnknown() || left.isNumber()) return HType.NUMBER;
+ if (left.isTypeUnknown() || left.isNumber()) {
+ return HType.INTEGER_OR_DOUBLE;
+ }
}
return HType.UNKNOWN;
}
@@ -1959,7 +2005,9 @@ class HEquals extends HRelational {
// speculatively test for all possible types. Therefore we try to match
// the two types. That is, if we see x == 3, then we speculatively test
// if x is a number and bailout if it isn't.
- if (right.isNumber()) return HType.NUMBER; // No need to be more precise.
+ // If right is a number we don't need more than a number (no need to match
+ // the exact type of right).
+ if (right.isNumber()) return HType.INTEGER_OR_DOUBLE;
// String equality testing is much more common than array equality
// testing.
if (right.isStringOrArray()) return HType.STRING;
@@ -2128,7 +2176,7 @@ class HIndex extends HInvokeStatic {
return HType.UNKNOWN;
}
- bool get builtin() => receiver.isStringOrArray() && index.isInteger();
+ bool get builtin() => receiver.isIndexablePrimitive() && index.isInteger();
}
class HIndexAssign extends HInvokeStatic {
« no previous file with comments | « lib/compiler/implementation/ssa/codegen.dart ('k') | lib/compiler/implementation/ssa/optimize.dart » ('j') | lib/compiler/implementation/ssa/optimize.dart » ('J')

Powered by Google App Engine
This is Rietveld 408576698