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 88c8e364b15..3ae2c1bd8b3 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 @@ -431,7 +431,7 @@ class IrBuilderVisitor extends ast.Visitor /// Maps each field from this class or a superclass to its initial value. Map fieldValues = - {}; + {}; // -- Evaluate field initializers --- // Evaluate field initializers in constructor and super constructors. 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 b0af0b99253..4395c00995b 100644 --- a/pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart +++ b/pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart @@ -2011,23 +2011,25 @@ abstract class BlockVisitor { /// Visits block-level nodes in lexical pre-order. /// - /// The IR may be transformed during the traversal, but the currently - /// visited node should not be removed, as its 'body' pointer is needed - /// for the traversal. + /// Traversal continues at the original success for the current node, so: + /// - The current node can safely be removed. + /// - Nodes inserted immediately below the current node will not be seen. + /// - The body of the current node should not be moved/removed, as traversal + /// would otherwise continue into an orphaned or relocated node. static void traverseInPreOrder(FunctionDefinition root, BlockVisitor v) { List stack = []; void walkBlock(InteriorNode block) { v.visit(block); Expression node = block.body; - v.visit(node); - while (node.next != null) { + while (node != null) { if (node is LetCont) { stack.addAll(node.continuations); } else if (node is LetHandler) { stack.add(node.handler); } - node = node.next; + Expression next = node.next; v.visit(node); + node = next; } } walkBlock(root); diff --git a/pkg/compiler/lib/src/cps_ir/optimizers.dart b/pkg/compiler/lib/src/cps_ir/optimizers.dart index 599476fc77d..79780293312 100644 --- a/pkg/compiler/lib/src/cps_ir/optimizers.dart +++ b/pkg/compiler/lib/src/cps_ir/optimizers.dart @@ -26,6 +26,7 @@ export 'inline.dart' show Inliner; export 'eagerly_load_statics.dart' show EagerlyLoadStatics; export 'loop_invariant_branch.dart' show LoopInvariantBranchMotion; export 'duplicate_branch.dart' show DuplicateBranchEliminator; +export 'use_field_initializers.dart' show UseFieldInitializers; export 'parent_visitor.dart' show ParentVisitor; /// An optimization pass over the CPS IR. diff --git a/pkg/compiler/lib/src/cps_ir/use_field_initializers.dart b/pkg/compiler/lib/src/cps_ir/use_field_initializers.dart new file mode 100644 index 00000000000..a8944c62737 --- /dev/null +++ b/pkg/compiler/lib/src/cps_ir/use_field_initializers.dart @@ -0,0 +1,212 @@ +// Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. +library dart2js.cps_ir.use_field_initializers; + +import 'cps_ir_nodes.dart'; +import 'optimizers.dart'; +import '../elements/elements.dart'; +import '../js_backend/js_backend.dart'; + +/// Eliminates [SetField] instructions when the value can instead be passed into +/// the field initializer of a [CreateInstance] instruction. +/// +/// This compensates for a somewhat common pattern where fields are initialized +/// in the constructor body instead of using intializers. For example: +/// +/// class Foo { +/// var x, y; +/// Foo(x, y) { +/// this.x = x; +/// this.y = y; +/// } +/// } +/// +/// ==> (IR for Foo constructor) +/// +/// foo = new D.Foo(null, null); +/// foo.x = 'a'; +/// foo.y = 'b'; +/// +/// ==> (after this pass) +/// +/// foo = new D.Foo('a', 'b'); +// +// TODO(asgerf): Store forwarding and load elimination could most likely +// handle this more generally. +// +class UseFieldInitializers extends BlockVisitor implements Pass { + String get passName => 'Use field initializers'; + + final JavaScriptBackend backend; + + final Set unescaped = new Set(); + + /// Continuation bindings separating the current traversal position from an + /// unescaped [CreateInstance]. When [CreateInstance] is sunk, these + /// continuations must sink as well to ensure the object remains in scope + /// inside the bound continuations. + final List letConts = []; + + /// If non-null, the bindings in [letConts] should sink to immediately below + /// this node. + InteriorNode letContSinkTarget = null; + EscapeVisitor escapeVisitor; + + UseFieldInitializers(this.backend); + + void rewrite(FunctionDefinition node) { + escapeVisitor = new EscapeVisitor(this); + BlockVisitor.traverseInPreOrder(node, this); + } + + void escape(Reference ref) { + Definition def = ref.definition; + if (def is CreateInstance) { + unescaped.remove(def); + if (unescaped.isEmpty) { + sinkLetConts(); + letConts.clear(); + } + } + } + + void visitContinuation(Continuation node) { + endBasicBlock(); + } + void visitLetHandler(LetHandler node) { + endBasicBlock(); + } + void visitInvokeContinuation(InvokeContinuation node) { + endBasicBlock(); + } + void visitBranch(Branch node) { + endBasicBlock(); + } + void visitRethrow(Rethrow node) { + endBasicBlock(); + } + void visitThrow(Throw node) { + endBasicBlock(); + } + void visitUnreachable(Unreachable node) { + endBasicBlock(); + } + + void visitLetMutable(LetMutable node) { + escape(node.value); + } + + void visitLetCont(LetCont node) { + if (unescaped.isNotEmpty) { + // Ensure we do not lift a LetCont if there is a sink target set above + // the current node. + sinkLetConts(); + letConts.add(node); + } + } + + void sinkLetConts() { + if (letContSinkTarget != null) { + for (LetCont letCont in letConts.reversed) { + letCont..remove()..insertBelow(letContSinkTarget); + } + letContSinkTarget = null; + } + } + + void endBasicBlock() { + sinkLetConts(); + letConts.clear(); + unescaped.clear(); + } + + void visitLetPrim(LetPrim node) { + Primitive prim = node.primitive; + if (prim is CreateInstance) { + unescaped.add(prim); + prim.arguments.forEach(escape); + return; + } + if (unescaped.isEmpty) return; + if (prim is SetField) { + escape(prim.value); + Primitive object = prim.object.definition; + if (object is CreateInstance && unescaped.contains(object)) { + int index = getFieldIndex(object.classElement, prim.field); + if (index == -1) { + // This field is not initialized at creation time, so we cannot pull + // set SetField into the CreateInstance instruction. We have to + // leave the instruction here, and this counts as a use of the object. + escape(prim.object); + } else { + // Replace the field initializer with the new value. There are no uses + // of the object before this, so the old value cannot have been seen. + object.arguments[index].changeTo(prim.value.definition); + prim.destroy(); + // The right-hand side might not be in scope at the CreateInstance. + // Sink the creation down to this point. + rebindCreateInstanceAt(object, node); + letContSinkTarget = node; + } + } + return; + } + if (prim is GetField) { + // When reading the field of a newly created object, just use the initial + // value and destroy the GetField. This can unblock the other optimization + // since we remove a use of the object. + Primitive object = prim.object.definition; + if (object is CreateInstance && unescaped.contains(object)) { + int index = getFieldIndex(object.classElement, prim.field); + if (index == -1) { + escape(prim.object); + } else { + prim.replaceUsesWith(object.arguments[index].definition); + prim.destroy(); + node.remove(); + } + } + return; + } + escapeVisitor.visit(node.primitive); + } + + void rebindCreateInstanceAt(CreateInstance prim, LetPrim newBinding) { + removeBinding(prim); + newBinding.primitive = prim; + prim.parent = newBinding; + } + + /// Returns the index of [field] in the canonical initialization order in + /// [classElement], or -1 if the field is not initialized at creation time + /// for that class. + int getFieldIndex(ClassElement classElement, FieldElement field) { + // There is no stored map from a field to its index in a given class, so we + // have to iterate over all instance fields until we find it. + int current = -1, index = -1; + classElement.forEachInstanceField((host, currentField) { + if (!backend.isNativeOrExtendsNative(host)) { + ++current; + if (currentField == field) { + index = current; + } + } + }, includeSuperAndInjectedMembers: true); + return index; + } + + void removeBinding(Primitive prim) { + LetPrim node = prim.parent; + node.remove(); + } +} + +class EscapeVisitor extends DeepRecursiveVisitor { + final UseFieldInitializers main; + EscapeVisitor(this.main); + + processReference(Reference ref) { + main.escape(ref); + } +} diff --git a/pkg/compiler/lib/src/js_backend/codegen/task.dart b/pkg/compiler/lib/src/js_backend/codegen/task.dart index 6e0619bc044..89f48ea67ff 100644 --- a/pkg/compiler/lib/src/js_backend/codegen/task.dart +++ b/pkg/compiler/lib/src/js_backend/codegen/task.dart @@ -281,6 +281,7 @@ class CpsFunctionCompiler implements FunctionCompiler { applyCpsPass(new LoopInvariantBranchMotion(), cpsFunction); applyCpsPass(new ShrinkingReducer(), cpsFunction); applyCpsPass(new ScalarReplacer(compiler), cpsFunction); + applyCpsPass(new UseFieldInitializers(backend), cpsFunction); applyCpsPass(new MutableVariableEliminator(), cpsFunction); applyCpsPass(new RedundantJoinEliminator(), cpsFunction); applyCpsPass(new RedundantPhiEliminator(), cpsFunction); diff --git a/tests/compiler/dart2js/cps_ir/constructor_15_test.dart b/tests/compiler/dart2js/cps_ir/constructor_15_test.dart new file mode 100644 index 00000000000..d745e08e589 --- /dev/null +++ b/tests/compiler/dart2js/cps_ir/constructor_15_test.dart @@ -0,0 +1,15 @@ +// ---- AUTO-GENERATED ------------------- +// This file was autogenerated by running: +// +// dart path/to/up_to_date_test.dart update +// +// Do not edit this file by hand. +// --------------------------------------- + +library tests.compiler.dart2js.cps_ir.constructor_15.dart; + +import 'runner.dart'; + +main(args) { + runTest("constructor_15.dart", update: args.length > 0 && args[0] == "update"); +} diff --git a/tests/compiler/dart2js/cps_ir/expected/constructor_15.js b/tests/compiler/dart2js/cps_ir/expected/constructor_15.js new file mode 100644 index 00000000000..22c0973f22c --- /dev/null +++ b/tests/compiler/dart2js/cps_ir/expected/constructor_15.js @@ -0,0 +1,19 @@ +// Expectation for test: +// // Method to test: generative_constructor(A#) +// class A { +// var x, y, z; +// A(x, y) { +// this.x = x; +// this.y = y; +// this.z = this.x / 2; +// } +// } +// +// main() { +// print(new A(123, 'sdf').y); +// try {} finally {} // Do not inline into main. +// } + +function(x, y) { + return new V.A(x, y, x / 2); +} diff --git a/tests/compiler/dart2js/cps_ir/input/constructor_15.dart b/tests/compiler/dart2js/cps_ir/input/constructor_15.dart new file mode 100644 index 00000000000..898b04fa5f7 --- /dev/null +++ b/tests/compiler/dart2js/cps_ir/input/constructor_15.dart @@ -0,0 +1,14 @@ +// Method to test: generative_constructor(A#) +class A { + var x, y, z; + A(x, y) { + this.x = x; + this.y = y; + this.z = this.x / 2; + } +} + +main() { + print(new A(123, 'sdf').y); + try {} finally {} // Do not inline into main. +} diff --git a/tests/compiler/dart2js/cps_ir/update_all.dart b/tests/compiler/dart2js/cps_ir/update_all.dart index 5874a25bc49..59c0bc3629c 100644 --- a/tests/compiler/dart2js/cps_ir/update_all.dart +++ b/tests/compiler/dart2js/cps_ir/update_all.dart @@ -96,6 +96,7 @@ main(args) { runTest('constructor_12.dart', update: true); runTest('constructor_13.dart', update: true); runTest('constructor_14.dart', update: true); + runTest('constructor_15.dart', update: true); runTest('constructor_2.dart', update: true); runTest('constructor_3.dart', update: true); runTest('constructor_4.dart', update: true);