From eb7befc3d925cd7255cd9ac8dca7e9219a1eb1e0 Mon Sep 17 00:00:00 2001 From: Asger Feldthaus Date: Fri, 6 Nov 2015 11:29:26 +0100 Subject: [PATCH] dart2js cps: Propagate container types for lists. Container type masks depend on the allocation site of an object, but the allocation site "key" is an AST node. We need to assign types to nodes in the type propagation pass where the AST nodes are (and should be) inaccessible, so we cannot directly query the type inference from there. The key could be changed to an opaque "AllocationSiteID" object with no members (just a key), but for now, we're just preserving the type mask for container types from build-time. Also has some other type propagation improvements related to containers. BUG= R=kmillikin@google.com Review URL: https://codereview.chromium.org/1416723008 . --- pkg/compiler/lib/src/common/names.dart | 9 ++- .../lib/src/cps_ir/cps_ir_builder.dart | 12 ++-- .../lib/src/cps_ir/cps_ir_builder_task.dart | 24 +++++++- pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart | 16 +++++- .../lib/src/cps_ir/type_mask_system.dart | 17 +++--- .../lib/src/cps_ir/type_propagation.dart | 56 +++++++++++++------ .../js_backend_cps_ir_control_flow_test.dart | 17 ++---- .../js_backend_cps_ir_interceptors_test.dart | 11 ++-- 8 files changed, 109 insertions(+), 53 deletions(-) diff --git a/pkg/compiler/lib/src/common/names.dart b/pkg/compiler/lib/src/common/names.dart index 45c7b1f45e8..fa1812ced1d 100644 --- a/pkg/compiler/lib/src/common/names.dart +++ b/pkg/compiler/lib/src/common/names.dart @@ -38,6 +38,9 @@ class Identifiers { /// The name of the runtime type property on 'Object'. static const String runtimeType_ = 'runtimeType'; + + /// The name of the getter returning the size of containers and strings. + static const String length = 'length'; } /// [Name]s commonly used. @@ -67,6 +70,8 @@ class Names { static const Name INDEX_NAME = const PublicName("[]"); static const Name INDEX_SET_NAME = const PublicName("[]="); static const Name CALL_NAME = Names.call; + + static const Name length = const PublicName(Identifiers.length); } /// [Selector]s commonly used. @@ -101,6 +106,8 @@ class Selectors { new Selector.call(const PublicName("compareTo"), CallStructure.ONE_ARG); static final Selector equals = new Selector.binaryOperator('=='); + + static final Selector length = new Selector.getter(Names.length); } /// [Uri]s commonly used. @@ -123,4 +130,4 @@ class Uris { /// The URI for 'dart:_native_typed_data'. static final Uri dart__native_typed_data = new Uri(scheme: 'dart', path: '_native_typed_data'); -} \ No newline at end of file +} diff --git a/pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart b/pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart index 36f569f6a86..b9f26d23c09 100644 --- a/pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart +++ b/pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart @@ -748,9 +748,11 @@ class IrBuilder { /// Creates a non-constant list literal of the provided [type] and with the /// provided [values]. ir.Primitive buildListLiteral(InterfaceType type, - Iterable values) { + Iterable values, + {TypeMask allocationSiteType}) { assert(isOpen); - return addPrimitive(new ir.LiteralList(type, values.toList())); + return addPrimitive(new ir.LiteralList(type, values.toList(), + allocationSiteType: allocationSiteType)); } /// Creates a non-constant map literal of the provided [type] and with the @@ -2635,7 +2637,8 @@ class IrBuilder { CallStructure callStructure, DartType type, List arguments, - SourceInformation sourceInformation) { + SourceInformation sourceInformation, + {TypeMask allocationSiteType}) { assert(isOpen); Selector selector = new Selector(SelectorKind.CALL, element.memberName, callStructure); @@ -2653,7 +2656,8 @@ class IrBuilder { } return _continueWithExpression( (k) => new ir.InvokeConstructor( - type, element, selector, arguments, k, sourceInformation)); + type, element, selector, arguments, k, sourceInformation, + allocationSiteType: allocationSiteType)); } ir.Primitive buildTypeExpression(DartType type) { diff --git a/pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart b/pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart index c1b33e0c046..5a771812d4d 100644 --- a/pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart +++ b/pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart @@ -235,6 +235,16 @@ abstract class IrBuilderVisitor extends ast.Visitor return irBuilder.makeFunctionDefinition(); } + /// Returns the allocation site-specific type for a given allocation. + /// + /// Currently, it is an error to call this with anything that is not the + /// allocation site for a List object (a literal list or a call to one + /// of the List constructors). + TypeMask getAllocationSiteType(ast.Node node) { + return compiler.typesTask.getGuaranteedTypeOfNode( + elements.analyzedElement, node); + } + ir.Primitive visit(ast.Node node) => node.accept(this); // ## Statements ## @@ -634,7 +644,8 @@ abstract class IrBuilderVisitor extends ast.Visitor } List values = node.elements.nodes.mapToList(visit); InterfaceType type = elements.getType(node); - return irBuilder.buildListLiteral(type, values); + return irBuilder.buildListLiteral(type, values, + allocationSiteType: getAllocationSiteType(node)); } ir.Primitive visitLiteralMap(ast.LiteralMap node) { @@ -3278,12 +3289,21 @@ class JsIrBuilderVisitor extends IrBuilderVisitor { // Use default values from the effective target, not the immediate target. ConstructorElement target = constructor.effectiveTarget; arguments = normalizeStaticArguments(callStructure, target, arguments); + TypeMask allocationSiteType; + ast.Node send = node.send; + if (Elements.isFixedListConstructorCall(constructor, send, compiler) || + Elements.isGrowableListConstructorCall(constructor, send, compiler) || + Elements.isFilledListConstructorCall(constructor, send, compiler) || + Elements.isConstructorOfTypedArraySubclass(constructor, compiler)) { + allocationSiteType = getAllocationSiteType(send); + } return irBuilder.buildConstructorInvocation( target, callStructure, constructor.computeEffectiveTargetType(type), arguments, - sourceInformationBuilder.buildNew(node)); + sourceInformationBuilder.buildNew(node), + allocationSiteType: allocationSiteType); } @override diff --git a/pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart b/pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart index 9c56c480ae8..50e84bdfec2 100644 --- a/pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart +++ b/pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart @@ -590,12 +590,20 @@ class InvokeConstructor extends CallExpression { final Selector selector; final SourceInformation sourceInformation; + /// If non-null, this is an allocation site-specific type that is potentially + /// better than the inferred return type of [target]. + /// + /// In particular, container type masks depend on the allocation site and + /// can therefore not be inferred solely based on the call target. + TypeMask allocationSiteType; + InvokeConstructor(this.dartType, this.target, this.selector, List args, Continuation cont, - this.sourceInformation) + this.sourceInformation, + {this.allocationSiteType}) : arguments = _referenceList(args), continuation = new Reference(cont); @@ -1337,7 +1345,11 @@ class LiteralList extends Primitive { final InterfaceType dartType; final List> values; - LiteralList(this.dartType, List values) + /// If non-null, this is an allocation site-specific type for the list + /// created here. + TypeMask allocationSiteType; + + LiteralList(this.dartType, List values, {this.allocationSiteType}) : this.values = _referenceList(values); accept(Visitor visitor) => visitor.visitLiteralList(this); diff --git a/pkg/compiler/lib/src/cps_ir/type_mask_system.dart b/pkg/compiler/lib/src/cps_ir/type_mask_system.dart index ab29618d031..cfc7ecee758 100644 --- a/pkg/compiler/lib/src/cps_ir/type_mask_system.dart +++ b/pkg/compiler/lib/src/cps_ir/type_mask_system.dart @@ -14,6 +14,7 @@ import '../js_backend/js_backend.dart' show JavaScriptBackend; import '../types/types.dart'; import '../types/constants.dart' show computeTypeMask; import '../universe/selector.dart' show Selector; +import '../universe/call_structure.dart' show CallStructure; import '../world.dart' show World; enum AbstractBool { @@ -37,7 +38,10 @@ class TypeMaskSystem { TypeMask get mapType => inferrer.mapType; TypeMask get nonNullType => inferrer.nonNullType; TypeMask get nullType => inferrer.nullType; - TypeMask get extendableNativeListType => backend.extendableArrayType; + TypeMask get extendableArrayType => backend.extendableArrayType; + TypeMask get fixedArrayType => backend.fixedArrayType; + TypeMask get arrayType => + new TypeMask.nonNullSubclass(helpers.jsArrayClass, classWorld); TypeMask get uint31Type => inferrer.uint31Type; TypeMask get uint32Type => inferrer.uint32Type; @@ -257,25 +261,22 @@ class TypeMaskSystem { return t.satisfies(helpers.jsPositiveIntClass, classWorld); } - // TODO(sra): Find a better name. 'NativeList' is a bad name because there - // are many native classes in dart:html that implement List but are not (and - // should not be) included in this predicate. - bool isDefinitelyNativeList(TypeMask t, {bool allowNull: false}) { + bool isDefinitelyArray(TypeMask t, {bool allowNull: false}) { if (!allowNull && t.isNullable) return false; return t.nonNullable().satisfies(helpers.jsArrayClass, classWorld); } - bool isDefinitelyMutableNativeList(TypeMask t, {bool allowNull: false}) { + bool isDefinitelyMutableArray(TypeMask t, {bool allowNull: false}) { if (!allowNull && t.isNullable) return false; return t.nonNullable().satisfies(helpers.jsMutableArrayClass, classWorld); } - bool isDefinitelyFixedNativeList(TypeMask t, {bool allowNull: false}) { + bool isDefinitelyFixedArray(TypeMask t, {bool allowNull: false}) { if (!allowNull && t.isNullable) return false; return t.nonNullable().satisfies(helpers.jsFixedArrayClass, classWorld); } - bool isDefinitelyExtendableNativeList(TypeMask t, {bool allowNull: false}) { + bool isDefinitelyExtendableArray(TypeMask t, {bool allowNull: false}) { if (!allowNull && t.isNullable) return false; return t.nonNullable().satisfies(helpers.jsExtendableArrayClass, classWorld); diff --git a/pkg/compiler/lib/src/cps_ir/type_propagation.dart b/pkg/compiler/lib/src/cps_ir/type_propagation.dart index a32329fb6ea..9d1a33523cc 100644 --- a/pkg/compiler/lib/src/cps_ir/type_propagation.dart +++ b/pkg/compiler/lib/src/cps_ir/type_propagation.dart @@ -144,30 +144,30 @@ class ConstantPropagationLattice { typeSystem.isDefinitelyUint(value.type, allowNull: allowNull); } - bool isDefinitelyNativeList(AbstractValue value, + bool isDefinitelyArray(AbstractValue value, {bool allowNull: false}) { return value.isNothing || - typeSystem.isDefinitelyNativeList(value.type, allowNull: allowNull); + typeSystem.isDefinitelyArray(value.type, allowNull: allowNull); } - bool isDefinitelyMutableNativeList(AbstractValue value, + bool isDefinitelyMutableArray(AbstractValue value, {bool allowNull: false}) { return value.isNothing || - typeSystem.isDefinitelyMutableNativeList(value.type, + typeSystem.isDefinitelyMutableArray(value.type, allowNull: allowNull); } - bool isDefinitelyFixedNativeList(AbstractValue value, + bool isDefinitelyFixedArray(AbstractValue value, {bool allowNull: false}) { return value.isNothing || - typeSystem.isDefinitelyFixedNativeList(value.type, + typeSystem.isDefinitelyFixedArray(value.type, allowNull: allowNull); } - bool isDefinitelyExtendableNativeList(AbstractValue value, + bool isDefinitelyExtendableArray(AbstractValue value, {bool allowNull: false}) { return value.isNothing || - typeSystem.isDefinitelyExtendableNativeList(value.type, + typeSystem.isDefinitelyExtendableArray(value.type, allowNull: allowNull); } @@ -1277,15 +1277,15 @@ class TransformingVisitor extends DeepRecursiveVisitor { Primitive list = getDartReceiver(node); AbstractValue listValue = getValue(list); // Ensure that the object is a native list or null. - if (!lattice.isDefinitelyNativeList(listValue, allowNull: true)) { + if (!lattice.isDefinitelyArray(listValue, allowNull: true)) { return false; } bool isFixedLength = - lattice.isDefinitelyFixedNativeList(listValue, allowNull: true); + lattice.isDefinitelyFixedArray(listValue, allowNull: true); bool isMutable = - lattice.isDefinitelyMutableNativeList(listValue, allowNull: true); + lattice.isDefinitelyMutableArray(listValue, allowNull: true); bool isExtendable = - lattice.isDefinitelyExtendableNativeList(listValue, allowNull: true); + lattice.isDefinitelyExtendableArray(listValue, allowNull: true); SourceInformation sourceInfo = node.sourceInformation; Continuation cont = node.continuation.definition; switch (node.selector.name) { @@ -2329,7 +2329,7 @@ class TransformingVisitor extends DeepRecursiveVisitor { } else if (class_ == helpers.jsArrayClass) { // JSArray has compile-time subclasses like JSFixedArray, but should // still be considered "exact" if the input is any subclass of JSArray. - if (typeSystem.isDefinitelyNativeList(interceptedInputsNonNullable)) { + if (typeSystem.isDefinitelyArray(interceptedInputsNonNullable)) { node.clearFlag(Interceptor.NON_NULL_INTERCEPT_SUBCLASS); } } else { @@ -2394,6 +2394,8 @@ class TypePropagationVisitor implements Visitor { JavaScriptBackend get backend => typeSystem.backend; + dart2js.Compiler get compiler => backend.compiler; + World get classWorld => typeSystem.classWorld; AbstractValue get nothing => lattice.nothing; @@ -2631,6 +2633,19 @@ class TypePropagationVisitor implements Visitor { if (receiver.isNothing) { return; // And come back later. } + + // Constant fold known length of containers. + if (node.selector == Selectors.length) { + AbstractValue object = getValue(getDartReceiver(node)); + if (typeSystem.isDefinitelyIndexable(object.type, allowNull: true)) { + int length = typeSystem.getContainerLength(object.type.nonNullable()); + if (length != null) { + setResult(node, constantValue(new IntConstantValue(length)), + canReplace: !object.isNullable); + } + } + } + if (!node.selector.isOperator) { // TODO(jgruber): Handle known methods on constants such as String.length. setResult(node, lattice.getInvokeReturnType(node.selector, node.mask)); @@ -2857,7 +2872,11 @@ class TypePropagationVisitor implements Visitor { } void visitInvokeConstructor(InvokeConstructor node) { - setResult(node, nonConstant(typeSystem.getReturnType(node.target))); + if (node.allocationSiteType != null) { + setResult(node, nonConstant(node.allocationSiteType)); + } else { + setResult(node, nonConstant(typeSystem.getReturnType(node.target))); + } } void visitThrow(Throw node) { @@ -2954,9 +2973,11 @@ class TypePropagationVisitor implements Visitor { } void visitLiteralList(LiteralList node) { - // Constant lists are translated into (Constant ListConstant(...)) IR nodes, - // and thus LiteralList nodes are NonConst. - setValue(node, nonConstant(typeSystem.extendableNativeListType)); + if (node.allocationSiteType != null) { + setValue(node, nonConstant(node.allocationSiteType)); + } else { + setValue(node, nonConstant(typeSystem.extendableArrayType)); + } } void visitLiteralMap(LiteralMap node) { @@ -3167,6 +3188,7 @@ class AbstractValue { bool get isNonConst => (kind == NONCONST); bool get isNullConstant => kind == CONSTANT && constant.isNull; bool get isTrueConstant => kind == CONSTANT && constant.isTrue; + bool get isFalseConstant => kind == CONSTANT && constant.isFalse; bool get isNullable => kind != NOTHING && type.isNullable; bool get isDefinitelyNotNull => kind == NOTHING || !type.isNullable; diff --git a/tests/compiler/dart2js/js_backend_cps_ir_control_flow_test.dart b/tests/compiler/dart2js/js_backend_cps_ir_control_flow_test.dart index 2365d4fe979..2d9e07e1802 100644 --- a/tests/compiler/dart2js/js_backend_cps_ir_control_flow_test.dart +++ b/tests/compiler/dart2js/js_backend_cps_ir_control_flow_test.dart @@ -138,12 +138,9 @@ main() { } }""",r""" function() { - var list = [1, 2, 3, 4, 5, 6], $length = list.length, i = 0; - for (; i < list.length; i = i + 1) { + var list = [1, 2, 3, 4, 5, 6], i = 0; + for (; i < 6; i = i + 1) P.print(list[i]); - if ($length !== list.length) - H.throwConcurrentModificationError(list); - } }"""), const TestEntry(""" main() { @@ -155,18 +152,14 @@ main() { } }""",r""" function() { - var xs = ["x", "y", "z"], ys = ["A", "B", "C"], $length = xs.length, length1 = ys.length, i = 0, i1 = 0, current, current1; - for (; i < xs.length; i = i + 1, i1 = i1 + 1) { + var xs = ["x", "y", "z"], ys = ["A", "B", "C"], i = 0, i1 = 0, current, current1; + for (; i < 3; i = i + 1, i1 = i1 + 1) { current = xs[i]; - if (length1 !== ys.length) - H.throwConcurrentModificationError(ys); - if (!(i1 < ys.length)) + if (!(i1 < 3)) break; current1 = ys[i1]; P.print(current); P.print(current1); - if ($length !== xs.length) - H.throwConcurrentModificationError(xs); } }"""), ]; diff --git a/tests/compiler/dart2js/js_backend_cps_ir_interceptors_test.dart b/tests/compiler/dart2js/js_backend_cps_ir_interceptors_test.dart index 947d91956be..4de37b15450 100644 --- a/tests/compiler/dart2js/js_backend_cps_ir_interceptors_test.dart +++ b/tests/compiler/dart2js/js_backend_cps_ir_interceptors_test.dart @@ -33,14 +33,11 @@ main() { }""", r""" function() { - var l = ["hest", ["h", "e", "s", "t"]], i = 0, x_, x, j; - for (P.print(l.length); i < l.length; i = i + 1) { - x_ = J.getInterceptor$as(x = l[i]); - for (j = 0; j < x_.get$length(x); j = j + 1) { - if (j >= x.length) - H.ioore(x, j); + var l = ["hest", ["h", "e", "s", "t"]], i = 0, x, j; + for (P.print(2); i < 2; i = i + 1) { + x = l[i]; + for (j = 0; j < x.length; j = j + 1) P.print(x[j]); - } } }"""), ];