From aaa180d67b0fb72dd558bbeea46bff8a0c0e4159 Mon Sep 17 00:00:00 2001 From: "ngeoffray@google.com" Date: Wed, 12 Dec 2012 11:55:09 +0000 Subject: [PATCH] Move the handling of operator[] into the new interceptors. Review URL: https://codereview.chromium.org//11348316 git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@16032 260f80e4-7a28-3924-810f-c04153c831b5 --- .../implementation/js_backend/backend.dart | 6 ++ .../compiler/implementation/lib/js_array.dart | 6 ++ .../implementation/lib/js_string.dart | 6 ++ .../implementation/native_handler.dart | 42 +++++------- .../compiler/implementation/ssa/bailout.dart | 11 ++- .../compiler/implementation/ssa/builder.dart | 27 +++++--- .../compiler/implementation/ssa/codegen.dart | 12 ++-- .../compiler/implementation/ssa/nodes.dart | 67 ++++++++++--------- .../compiler/implementation/ssa/optimize.dart | 23 +++---- .../compiler/implementation/ssa/tracer.dart | 7 +- tests/co19/co19-dart2js.status | 2 + tests/compiler/dart2js/mock_compiler.dart | 1 + 12 files changed, 116 insertions(+), 94 deletions(-) diff --git a/sdk/lib/_internal/compiler/implementation/js_backend/backend.dart b/sdk/lib/_internal/compiler/implementation/js_backend/backend.dart index 902414dbf26..c48fb27ad8f 100644 --- a/sdk/lib/_internal/compiler/implementation/js_backend/backend.dart +++ b/sdk/lib/_internal/compiler/implementation/js_backend/backend.dart @@ -849,6 +849,12 @@ class JavaScriptBackend extends Backend { addInterceptors(jsStringClass, enqueuer); } else if (cls == compiler.listClass) { addInterceptors(jsArrayClass, enqueuer); + // The backend will try to optimize array access and use the + // `ioore` and `iae` helpers directly. + enqueuer.registerStaticUse( + compiler.findHelper(const SourceString('ioore'))); + enqueuer.registerStaticUse( + compiler.findHelper(const SourceString('iae'))); } else if (cls == compiler.intClass) { addInterceptors(jsIntClass, enqueuer); addInterceptors(jsNumberClass, enqueuer); diff --git a/sdk/lib/_internal/compiler/implementation/lib/js_array.dart b/sdk/lib/_internal/compiler/implementation/lib/js_array.dart index adbea3c8e83..c9987754645 100644 --- a/sdk/lib/_internal/compiler/implementation/lib/js_array.dart +++ b/sdk/lib/_internal/compiler/implementation/lib/js_array.dart @@ -181,4 +181,10 @@ class JSArray implements List { checkGrowable(this, 'set length'); JS('void', r'#.length = #', this, newLength); } + + E operator [](int index) { + if (index is !int) throw new ArgumentError(index); + if (index >= length || index < 0) throw new RangeError.value(index); + return JS('var', '#[#]', this, index); + } } diff --git a/sdk/lib/_internal/compiler/implementation/lib/js_string.dart b/sdk/lib/_internal/compiler/implementation/lib/js_string.dart index 48a2d0107d1..3b5619819ce 100644 --- a/sdk/lib/_internal/compiler/implementation/lib/js_string.dart +++ b/sdk/lib/_internal/compiler/implementation/lib/js_string.dart @@ -163,4 +163,10 @@ class JSString implements String { Type get runtimeType => String; int get length => JS('int', r'#.length', this); + + String operator [](int index) { + if (index is !int) throw new ArgumentError(index); + if (index >= length || index < 0) throw new RangeError.value(index); + return JS('String', '#[#]', this, index); + } } diff --git a/sdk/lib/_internal/compiler/implementation/native_handler.dart b/sdk/lib/_internal/compiler/implementation/native_handler.dart index 0d30ecd64fd..ef93f454c05 100644 --- a/sdk/lib/_internal/compiler/implementation/native_handler.dart +++ b/sdk/lib/_internal/compiler/implementation/native_handler.dart @@ -104,6 +104,11 @@ abstract class NativeEnqueuerBase implements NativeEnqueuer { void processNativeClasses(Collection libraries) { libraries.forEach(processNativeClassesInLibrary); + processNativeClass(compiler.listClass); + processNativeClass(compiler.stringClass); + processNativeClass(compiler.intClass); + processNativeClass(compiler.doubleClass); + processNativeClass(compiler.nullClass); if (!enableLiveTypeAnalysis) { nativeClasses.forEach((c) => enqueueClass(c, 'forced')); flushQueue(); @@ -113,19 +118,19 @@ abstract class NativeEnqueuerBase implements NativeEnqueuer { void processNativeClassesInLibrary(LibraryElement library) { // Use implementation to ensure the inclusion of injected members. library.implementation.forEachLocalMember((Element element) { - if (element.kind == ElementKind.CLASS) { - ClassElement classElement = element; - if (classElement.isNative()) { - nativeClasses.add(classElement); - unusedClasses.add(classElement); - - // Resolve class to ensure the class has valid inheritance info. - classElement.ensureResolved(compiler); - } + if (element.isClass() && element.isNative()) { + processNativeClass(element); } }); } + void processNativeClass(ClassElement classElement) { + nativeClasses.add(classElement); + unusedClasses.add(classElement); + // Resolve class to ensure the class has valid inheritance info. + classElement.ensureResolved(compiler); + } + ClassElement get annotationCreatesClass { findAnnotationClasses(); return _annotationCreatesClass; @@ -311,27 +316,12 @@ abstract class NativeEnqueuerBase implements NativeEnqueuer { matchedTypeConstraints.add(type); if (type is SpecialType) { if (type == SpecialType.JsArray) { - world.registerInstantiatedClass(compiler.listClass); + enqueueClass(compiler.listClass, 'core type'); } else if (type == SpecialType.JsObject) { - world.registerInstantiatedClass(compiler.objectClass); + enqueueClass(compiler.objectClass, 'core type'); } continue; } - if (type is InterfaceType) { - if (type.element == compiler.intClass) { - world.registerInstantiatedClass(compiler.intClass); - } else if (type.element == compiler.doubleClass) { - world.registerInstantiatedClass(compiler.doubleClass); - } else if (type.element == compiler.numClass) { - world.registerInstantiatedClass(compiler.numClass); - } else if (type.element == compiler.stringClass) { - world.registerInstantiatedClass(compiler.stringClass); - } else if (type.element == compiler.nullClass) { - world.registerInstantiatedClass(compiler.nullClass); - } else if (type.element == compiler.boolClass) { - world.registerInstantiatedClass(compiler.boolClass); - } - } assert(type is DartType); enqueueUnusedClassesMatching( (nativeClass) => compiler.types.isSubtype(nativeClass.thisType, type), diff --git a/sdk/lib/_internal/compiler/implementation/ssa/bailout.dart b/sdk/lib/_internal/compiler/implementation/ssa/bailout.dart index 75bbdda219b..8a018c193d3 100644 --- a/sdk/lib/_internal/compiler/implementation/ssa/bailout.dart +++ b/sdk/lib/_internal/compiler/implementation/ssa/bailout.dart @@ -139,6 +139,12 @@ class SsaTypeGuardInserter extends HGraphVisitor implements OptimizationPhase { if (isNested(userLoopHeader, currentLoopHeader)) return true; } + bool isIndexOperatorOnIndexablePrimitive(instruction, types) { + return instruction is HIndex + || (instruction is HInvokeDynamicMethod + && instruction.isIndexOperatorOnIndexablePrimitive(types)); + } + // To speed up computations on values loaded from arrays, we // insert type guards for builtin array indexing operations in // nested loops. Since this can blow up code size quite @@ -146,9 +152,8 @@ class SsaTypeGuardInserter extends HGraphVisitor implements OptimizationPhase { // inserted for this method. The code size price for an additional // type guard is much smaller than the first one that causes the // generation of a bailout method. - if (instruction is HIndex && - (instruction as HIndex).isBuiltin(types) && - hasTypeGuards) { + if (hasTypeGuards + && isIndexOperatorOnIndexablePrimitive(instruction, types)) { HBasicBlock loopHeader = instruction.block.enclosingLoopHeader; if (loopHeader != null && loopHeader.parentLoopHeader != null) { return true; diff --git a/sdk/lib/_internal/compiler/implementation/ssa/builder.dart b/sdk/lib/_internal/compiler/implementation/ssa/builder.dart index ebbec8c35c4..10ea4a889bb 100644 --- a/sdk/lib/_internal/compiler/implementation/ssa/builder.dart +++ b/sdk/lib/_internal/compiler/implementation/ssa/builder.dart @@ -2702,13 +2702,7 @@ class SsaBuilder extends ResolvedVisitor implements Visitor { Operator op = node.selector; if (const SourceString("[]") == op.source) { - HStatic target = new HStatic(interceptors.getIndexInterceptor()); - add(target); - visit(node.receiver); - HInstruction receiver = pop(); - visit(node.argumentsNode); - HInstruction index = pop(); - push(new HIndex(target, receiver, index)); + visitDynamicSend(node); } else if (const SourceString("&&") == op.source || const SourceString("||") == op.source) { visitLogicalAndOr(node, op); @@ -3592,6 +3586,19 @@ class SsaBuilder extends ResolvedVisitor implements Visitor { } } + HInvokeDynamicMethod buildInvokeDynamicWithOneArgument( + Node node, Selector selector, HInstruction receiver, HInstruction arg0) { + Set interceptedClasses = + getInterceptedClassesOn(node, selector); + List inputs = []; + if (interceptedClasses != null) { + inputs.add(invokeInterceptor(interceptedClasses, receiver, node)); + } + inputs.add(receiver); + inputs.add(arg0); + return new HInvokeDynamicMethod(selector, inputs); + } + visitSendSet(SendSet node) { Element element = elements[node]; if (!Elements.isUnresolved(element) && element.impliesType()) { @@ -3643,9 +3650,9 @@ class SsaBuilder extends ResolvedVisitor implements Visitor { index = pop(); value = graph.addConstantInt(1, constantSystem); } - HStatic indexMethod = new HStatic(interceptors.getIndexInterceptor()); - add(indexMethod); - HInstruction left = new HIndex(indexMethod, receiver, index); + + HInvokeDynamicMethod left = buildInvokeDynamicWithOneArgument( + node, new Selector.index(), receiver, index); add(left); Element opElement = elements[op]; visitBinary(left, op, value); diff --git a/sdk/lib/_internal/compiler/implementation/ssa/codegen.dart b/sdk/lib/_internal/compiler/implementation/ssa/codegen.dart index bf500a8d468..fe8c5eeece7 100644 --- a/sdk/lib/_internal/compiler/implementation/ssa/codegen.dart +++ b/sdk/lib/_internal/compiler/implementation/ssa/codegen.dart @@ -2166,14 +2166,10 @@ abstract class SsaCodeGenerator implements HVisitor, HBlockInformationVisitor { } void visitIndex(HIndex node) { - if (node.isBuiltin(types)) { - use(node.inputs[1]); - js.Expression receiver = pop(); - use(node.inputs[2]); - push(new js.PropertyAccess(receiver, pop()), node); - } else { - visitInvokeStatic(node); - } + use(node.receiver); + js.Expression receiver = pop(); + use(node.index); + push(new js.PropertyAccess(receiver, pop()), node); } void visitIndexAssign(HIndexAssign node) { diff --git a/sdk/lib/_internal/compiler/implementation/ssa/nodes.dart b/sdk/lib/_internal/compiler/implementation/ssa/nodes.dart index 290a3579d64..b847df4153f 100644 --- a/sdk/lib/_internal/compiler/implementation/ssa/nodes.dart +++ b/sdk/lib/_internal/compiler/implementation/ssa/nodes.dart @@ -289,7 +289,7 @@ class HBaseVisitor extends HGraphVisitor implements HVisitor { visitGreaterEqual(HGreaterEqual node) => visitRelational(node); visitIdentity(HIdentity node) => visitRelational(node); visitIf(HIf node) => visitConditionalBranch(node); - visitIndex(HIndex node) => visitInvokeStatic(node); + visitIndex(HIndex node) => visitInstruction(node); visitIndexAssign(HIndexAssign node) => visitInvokeStatic(node); visitIntegerCheck(HIntegerCheck node) => visitCheck(node); visitInterceptor(HInterceptor node) => visitInstruction(node); @@ -1320,8 +1320,34 @@ class HInvokeClosure extends HInvokeDynamic { class HInvokeDynamicMethod extends HInvokeDynamic { HInvokeDynamicMethod(Selector selector, List inputs) : super(selector, null, inputs); - toString() => 'invoke dynamic method: $selector'; + String toString() => 'invoke dynamic method: $selector'; accept(HVisitor visitor) => visitor.visitInvokeDynamicMethod(this); + + bool isIndexOperatorOnIndexablePrimitive(HTypeMap types) { + return isInterceptorCall + && selector.kind == SelectorKind.INDEX + && inputs[1].isIndexablePrimitive(types); + } + + HType computeDesiredTypeForInput(HInstruction input, + HTypeMap types, + Compiler compiler) { + // TODO(ngeoffray): Move this logic into a different class that + // will know what type it wants for a given selector. + if (selector.kind != SelectorKind.INDEX) return HType.UNKNOWN; + if (!isInterceptorCall) return HType.UNKNOWN; + + HInstruction index = inputs[2]; + if (input == inputs[1] && + (index.isTypeUnknown(types) || index.isNumber(types))) { + return HType.INDEXABLE_PRIMITIVE; + } + // The index should be an int when the receiver is a string or array. + // However it turns out that inserting an integer check in the optimized + // version is cheaper than having another bailout case. This is true, + // because the integer check will simply throw if it fails. + return HType.UNKNOWN; + } } abstract class HInvokeDynamicField extends HInvokeDynamic { @@ -2439,41 +2465,20 @@ class HLiteralList extends HInstruction { } } -class HIndex extends HInvokeStatic { - HIndex(HStatic target, HInstruction receiver, HInstruction index) - : super([target, receiver, index]); - toString() => 'index operator'; +class HIndex extends HInstruction { + HIndex(HInstruction receiver, HInstruction index) + : super([receiver, index]); + String toString() => 'index operator'; accept(HVisitor visitor) => visitor.visitIndex(this); void prepareGvn(HTypeMap types) { clearAllSideEffects(); - if (isBuiltin(types)) { - setDependsOnIndexStore(); - setUseGvn(); - } else { - setAllSideEffects(); - } + setDependsOnIndexStore(); + setUseGvn(); } - HInstruction get receiver => inputs[1]; - HInstruction get index => inputs[2]; - - HType computeDesiredTypeForNonTargetInput(HInstruction input, - HTypeMap types, - Compiler compiler) { - if (input == receiver && - (index.isTypeUnknown(types) || index.isNumber(types))) { - return HType.INDEXABLE_PRIMITIVE; - } - // The index should be an int when the receiver is a string or array. - // However it turns out that inserting an integer check in the optimized - // version is cheaper than having another bailout case. This is true, - // because the integer check will simply throw if it fails. - return HType.UNKNOWN; - } - - bool isBuiltin(HTypeMap types) - => receiver.isIndexablePrimitive(types) && index.isInteger(types); + HInstruction get receiver => inputs[0]; + HInstruction get index => inputs[1]; int typeCode() => HInstruction.INDEX_TYPECODE; bool typeEquals(HInstruction other) => other is HIndex; diff --git a/sdk/lib/_internal/compiler/implementation/ssa/optimize.dart b/sdk/lib/_internal/compiler/implementation/ssa/optimize.dart index b6ea705a87a..cbc32d9205e 100644 --- a/sdk/lib/_internal/compiler/implementation/ssa/optimize.dart +++ b/sdk/lib/_internal/compiler/implementation/ssa/optimize.dart @@ -206,8 +206,7 @@ class SsaConstantFolder extends HBaseVisitor implements OptimizationPhase { return node; } - HInstruction handleInterceptorCall(HInvokeDynamic node) { - if (node is !HInvokeDynamicMethod) return null; + HInstruction handleInterceptorCall(HInvokeDynamicMethod node) { HInstruction input = node.inputs[1]; if (input.isString(types) && node.selector.name == const SourceString('toString')) { @@ -229,6 +228,11 @@ class SsaConstantFolder extends HBaseVisitor implements OptimizationPhase { } Selector selector = node.selector; + + if (node.isIndexOperatorOnIndexablePrimitive(types)) { + return new HIndex(node.inputs[1], node.inputs[2]); + } + SourceString selectorName = selector.name; Element target; if (input.isExtendableArray(types)) { @@ -277,7 +281,7 @@ class SsaConstantFolder extends HBaseVisitor implements OptimizationPhase { return node; } - HInstruction visitInvokeDynamic(HInvokeDynamic node) { + HInstruction visitInvokeDynamicMethod(HInvokeDynamicMethod node) { if (node.isInterceptorCall) return handleInterceptorCall(node); HType receiverType = types[node.receiver]; if (receiverType.isExact()) { @@ -304,7 +308,7 @@ class SsaConstantFolder extends HBaseVisitor implements OptimizationPhase { * [HInvokeDynamic] because we know the receiver is not a JS * primitive object. */ - HInstruction fromPrimitiveInstructionToDynamicInvocation(HInvokeStatic node, + HInstruction fromPrimitiveInstructionToDynamicInvocation(HInstruction node, Selector selector) { HBoundedType type = types[node.inputs[1]]; HInvokeDynamicMethod result = new HInvokeDynamicMethod( @@ -331,15 +335,6 @@ class SsaConstantFolder extends HBaseVisitor implements OptimizationPhase { return node; } - - HInstruction visitIndex(HIndex node) { - if (!node.receiver.canBePrimitive(types)) { - Selector selector = new Selector.index(); - return fromPrimitiveInstructionToDynamicInvocation(node, selector); - } - return node; - } - HInstruction visitIndexAssign(HIndexAssign node) { if (!node.receiver.canBePrimitive(types)) { Selector selector = new Selector.indexSet(); @@ -803,7 +798,6 @@ class SsaCheckInserter extends HBaseVisitor implements OptimizationPhase { } void visitIndex(HIndex node) { - if (!node.receiver.isIndexablePrimitive(types)) return; if (boundsChecked.contains(node)) return; HInstruction index = node.index; if (!node.index.isInteger(types)) { @@ -811,7 +805,6 @@ class SsaCheckInserter extends HBaseVisitor implements OptimizationPhase { } index = insertBoundsCheck(node, node.receiver, index); node.changeUse(node.index, index); - assert(node.isBuiltin(types)); } void visitIndexAssign(HIndexAssign node) { diff --git a/sdk/lib/_internal/compiler/implementation/ssa/tracer.dart b/sdk/lib/_internal/compiler/implementation/ssa/tracer.dart index e724bbb0dd7..0b468401e7c 100644 --- a/sdk/lib/_internal/compiler/implementation/ssa/tracer.dart +++ b/sdk/lib/_internal/compiler/implementation/ssa/tracer.dart @@ -306,7 +306,12 @@ class HInstructionStringifier implements HVisitor { return "$invokeType: $functionName($argumentsString)"; } - String visitIndex(HIndex node) => visitInvokeStatic(node); + String visitIndex(HIndex node) { + String receiver = temporaryId(node.receiver); + String index = temporaryId(node.index); + return "Index: $receiver[$index]"; + } + String visitIndexAssign(HIndexAssign node) => visitInvokeStatic(node); String visitIntegerCheck(HIntegerCheck node) { diff --git a/tests/co19/co19-dart2js.status b/tests/co19/co19-dart2js.status index 3ad85ec629a..e33dee97f84 100644 --- a/tests/co19/co19-dart2js.status +++ b/tests/co19/co19-dart2js.status @@ -527,6 +527,8 @@ Language/11_Expressions/30_Identifier_Reference_A07_t01: Fail # Checks that it i Language/12_Statements/03_Variable_Declaration_A04_t01: Fail # Checks that if the variable declaration is prefixed with the const modifier, then variable must be initialized to a constant expression. Language/15_Reference/1_Lexical_Rules_A02_t06: Fail # Checks that Unicode whitespaces other than WHITESPACE are not permitted in the source code. Checks symbol U+00a0. +Language/07_Classes/1_Instance_Methods/2_Operators_A04_t15: Fail # http://dartbug.com/7149 +Language/07_Classes/1_Instance_Methods/2_Operators_A04_t16: Fail # http://dartbug.com/7149 # # Unexpected compile-time errors. diff --git a/tests/compiler/dart2js/mock_compiler.dart b/tests/compiler/dart2js/mock_compiler.dart index 883f02be487..48b9e27ba0e 100644 --- a/tests/compiler/dart2js/mock_compiler.dart +++ b/tests/compiler/dart2js/mock_compiler.dart @@ -51,6 +51,7 @@ const String DEFAULT_HELPERLIB = r''' const String DEFAULT_INTERCEPTORSLIB = r''' class JSArray { var length; + operator[](index) {} } class JSString { var length;