Change the type inference for fields in dart2js

lib/compiler/implementation/ssa/optimize.dart

Index: lib/compiler/implementation/ssa/optimize.dart
diff --git a/lib/compiler/implementation/ssa/optimize.dart b/lib/compiler/implementation/ssa/optimize.dart
index b019498dbf3ad920d500bb87f646ccb58298080c..c34d425972aa982aac94358ac9bbd33ebf93ca72 100644
--- a/lib/compiler/implementation/ssa/optimize.dart
+++ b/lib/compiler/implementation/ssa/optimize.dart
@@ -47,7 +47,7 @@ class SsaOptimizerTask extends CompilerTask {
           // opportunities for constant folding.
           new SsaConstantFolder(constantSystem, backend, work, types),
           new SsaDeadCodeEliminator(types),
-          new SsaRegisterRecompilationCandidates(backend, work, types)];
+          new SsaConstructionFieldTypes(backend, work, types)];
       runPhases(graph, phases);
     });
   }
@@ -62,7 +62,6 @@ class SsaOptimizerTask extends CompilerTask {
     return measure(() {
       // Run the phases that will generate type guards.
       List<OptimizationPhase> phases = <OptimizationPhase>[
-          new SsaRecompilationFieldTypePropagator(backend, work, types),
           new SsaSpeculativeTypePropagator(compiler, types),
           new SsaTypeGuardInserter(compiler, work, types),
           new SsaEnvironmentBuilder(compiler),
@@ -76,7 +75,8 @@ class SsaOptimizerTask extends CompilerTask {
           // Then run the [SsaCheckInserter] because the type propagator also
           // propagated types non-speculatively. For example, it might have
           // propagated the type array for a call to the List constructor.
-          new SsaCheckInserter(backend, types, context.boundsChecked)];
+          new SsaCheckInserter(backend, types, context.boundsChecked),
+          new SsaConstructionFieldTypes(backend, work, types)];

[ngeoffray, 2012/09/24 15:39:29]

That's not right, the speculative optimization should not run this phase. My
comment was that this phase should be run only once, and since you add it to the
list line 50, then it will be run twice (normal optimizer and speculative
optimizer share the same list).

[Søren Gjesse, 2012/09/25 13:33:21]

Sorry my bad. As discussed offline added boolean argument to optimize.'to
indicate whether this is the speculative phase.

       runPhases(graph, phases);
       return !work.guards.isEmpty();
     });
@@ -612,23 +612,15 @@ class SsaConstantFolder extends HBaseVisitor implements OptimizationPhase {
       // initializer list.
       isFinalOrConst = true;
     }
-    if (!isFinalOrConst &&
-        !compiler.codegenWorld.hasInvokedSetter(field, compiler) &&
-        !compiler.codegenWorld.hasFieldSetter(field, compiler)) {
-      switch (compiler.phase) {
-        case Compiler.PHASE_COMPILING:
-          compiler.enqueuer.codegen.registerRecompilationCandidate(
-              work.element);
-          break;
-        case Compiler.PHASE_RECOMPILING:
-          // If field is not final or const but no setters are used then the
-          // field might be considered final anyway as it will be either
-          // un-initialized or initialized in the constructor initializer list.
-          isFinalOrConst = true;
-          break;
-      }
+    HFieldGet result = new HFieldGet(
+        field, node.inputs[0], isAssignable: !isFinalOrConst);
+    HType type = backend.optimisticFieldType(field);
+    if (type != null) {
+      result.guaranteedType = type;
+      backend.registerFieldTypesOptimization(
+          work.element, field, result.guaranteedType);
     }
-    return new HFieldGet(field, node.inputs[0], isAssignable: !isFinalOrConst);
+    return result;
   }
 
   HInstruction visitInvokeDynamicSetter(HInvokeDynamicSetter node) {
@@ -1220,159 +1212,126 @@ class SsaTypeConversionInserter extends HBaseVisitor
 }
 
 
-// Base class for the handling of recompilation based on inferred
-// field types.
-class BaseRecompilationVisitor extends HBaseVisitor {
+// Analyze the constructors to see if some fields will always have a specific
+// type after construction. If this is the case we can ignore the type given
+// by the field initializer. This is especially useful when the field
+// initializer is initializing the field to null.
+class SsaConstructionFieldTypes
+    extends HBaseVisitor implements OptimizationPhase {
   final JavaScriptBackend backend;
   final WorkItem work;
   final HTypeMap types;
-  Compiler get compiler => backend.compiler;
+  final String name = "SsaConstructionFieldTypes";
+  final Set<HInstruction> thisUsers;
+  final Set<Element> allSetters;
+  final Map<HBasicBlock, Map<Element, HType>> blockFieldSetters;
+  bool thisExposed = false;
+  HGraph currentGraph;
+  Map<Element, HType> currentFieldSetters;
+
+  SsaConstructionFieldTypes(JavaScriptBackend this.backend,
+         WorkItem this.work,
+         HTypeMap this.types)
+      : thisUsers = new Set<HInstruction>(),
+        allSetters = new Set<Element>(),
+        blockFieldSetters = new Map<HBasicBlock, Map<Element, HType>>();
 
-  BaseRecompilationVisitor(this.backend, this.work, this.types);
-
-  abstract void handleFieldGet(HFieldGet node, HType type);
-  abstract void handleFieldNumberOperation(HFieldGet field, HType type);
-
-  // Checks if the binary invocation operates on a field and a
-  // constant number. If it does [handleFieldNumberOperation] is
-  // called with the field and the type inferred for the field so far.
-  void checkFieldNumberOperation(HInvokeBinary node) {
-    // Determine if one of the operands is an HFieldGet.
-    HFieldGet field;
-    HInstruction other;
-    if (node.left is HFieldGet) {
-      field = node.left;
-      other = node.right;
-    } else if (node.right is HFieldGet) {
-      field = node.right;
-      other = node.left;
-    }
-    // Try to optimize the case where a field which is known to always
-    // be an integer is compared with a constant number.
-    if (other != null &&
-        other.isConstantNumber() &&
-        field.element != null &&
-        field.element.isMember()) {
-      // Calculate the field type from the information available.  If
-      // we have type information for the field and it contains NUMBER
-      // we use it as a candidate for recompilation.
-      Element fieldElement = field.element;
-      HType fieldSettersType = backend.fieldSettersTypeSoFar(fieldElement);
-      HType initializersType = backend.typeFromInitializersSoFar(fieldElement);
-      HType fieldType = fieldSettersType.union(initializersType);
-      HType type = HType.NUMBER.union(fieldType);
-      if (type == HType.NUMBER) {
-        handleFieldNumberOperation(field, fieldType);
-      }
-    }
+  void visitGraph(HGraph graph) {
+    currentGraph = graph;
+    if (!work.element.isGenerativeConstructorBody() &&
+        !work.element.isGenerativeConstructor()) return;
+    visitDominatorTree(graph);
   }
 
-  void visitFieldGet(HFieldGet node) {
-    if (!node.element.isInstanceMember()) return;
-    Element field = node.element;
-    if (field != null) {
-      HType type = backend.optimisticFieldTypeAfterConstruction(field);
-      if (!type.isUnknown()) {
-        // Allow handling even if we haven't seen any types for this
-        // field yet. There might still be only one setter in an
-        // initializer list or constructor body and recompilation
-        // can therefore pay off.
-        handleFieldGet(node, type);
+  visitBasicBlock(HBasicBlock block) {
+    if (block.predecessors.length == 0) {
+      // Create a new empty map for the first block.
+      currentFieldSetters = new Map<Element, HType>();
+    } else {
+      // Build a map which intersects the fields from all predecessors. For
+      // each field in this intersection it unions the types.
+      currentFieldSetters =
+          new Map.from(blockFieldSetters[block.predecessors[0]]);
+      // Loop headers are the only nodes with back edges.
+      if (!block.isLoopHeader()) {
+        for (int i = 1; i < block.predecessors.length; i++) {
+          Map<Element, HType> predecessorsFieldSetters =
+              blockFieldSetters[block.predecessors[i]];
+          Map<Element, HType> newFieldSetters = new Map<Element, HType>();
+          predecessorsFieldSetters.forEach((Element element, HType type) {
+            HType currentType = currentFieldSetters[element];
+            if (currentType != null) {
+              newFieldSetters[element] = currentType.union(type);
+            } else {
+              newFieldSetters[element] = type;
+            }
+          });
+          currentFieldSetters = newFieldSetters;
+        }
+      } else {
+        Expect.isTrue(block.predecessors.length <= 2);
       }
     }
+    block.forEachInstruction(
+        (HInstruction instruction) => instruction.accept(this));
+    assert(currentFieldSetters != null);
+    blockFieldSetters[block] = currentFieldSetters;
   }
 
-  HInstruction visitEquals(HEquals node) {
-    checkFieldNumberOperation(node);
+  visitInstruction(HInstruction instruction) {
+    // All instructions not explicitly handled below will flag the this
+    // exposure if using this.
+    thisExposed = thisExposed || thisUsers.contains(instruction);
   }
 
-  HInstruction visitBinaryArithmetic(HBinaryArithmetic node) {
-    checkFieldNumberOperation(node);
-  }
-}
-
-
-// Visitor that registers candidates for recompilation.
-class SsaRegisterRecompilationCandidates
-    extends BaseRecompilationVisitor implements OptimizationPhase {
-  final String name = "SsaRegisterRecompileCandidates";
-  HGraph graph;
-
-  SsaRegisterRecompilationCandidates(JavaScriptBackend backend,
-                                     WorkItem work,
-                                     HTypeMap types)
-      : super(backend, work, types);
-
-  void visitGraph(HGraph visitee) {
-    graph = visitee;
-    if (compiler.phase == Compiler.PHASE_COMPILING) {
-      visitDominatorTree(visitee);
-    }
+  visitThis(HThis instruction) {
+    // Collect all users of this in a set to make the this exposed check simple
+    // and cheap.
+    instruction.usedBy.forEach(thisUsers.add);
   }
 
-  void handleFieldGet(HFieldGet node, HType type) {
-    assert(compiler.phase == Compiler.PHASE_COMPILING);
-    compiler.enqueuer.codegen.registerRecompilationCandidate(
-        work.element);
+  visitFieldGet(HInstruction _) {
+    // The field get instruction is allowed to use this.
   }
 
-  void handleFieldNumberOperation(HFieldGet node, HType type) {
-    assert(compiler.phase == Compiler.PHASE_COMPILING);
-    compiler.enqueuer.codegen.registerRecompilationCandidate(
-        work.element);
+  visitForeignNew(HForeignNew node) {
+    // The HForeignNew instruction is used in the generative constructor to
+    // initialize all fields in newly created objects. The fields are
+    // initialized to the value present in the initializer list or set to null
+    // if not otherwise initialized.
+    int j = 0;
+    node.element.forEachInstanceField(
+      includeBackendMembers: true,
+      includeSuperMembers: true,
+      f: (ClassElement enclosingClass, Element element) {
+        backend.registerFieldInitializer(element, types[node.inputs[j]]);
+        j++;
+      });
   }
-}
-
-
-// Visitor that sets the known or suspected type of fields during
-// recompilation.
-class SsaRecompilationFieldTypePropagator
-    extends BaseRecompilationVisitor implements OptimizationPhase {
-  final String name = "SsaRecompilationFieldTypePropagator";
-  HGraph graph;
-
-  SsaRecompilationFieldTypePropagator(JavaScriptBackend backend,
-                                      WorkItem work,
-                                      HTypeMap types)
-      : super(backend, work, types);
 
-  void visitGraph(HGraph visitee) {
-    graph = visitee;
-    if (compiler.phase == Compiler.PHASE_RECOMPILING) {
-      visitDominatorTree(visitee);
+  visitFieldSet(HFieldSet node) {
+    Element field = node.element;
+    HInstruction value = node.value;
+    HType type = types[value];
+    allSetters.add(field);
+    if (work.element.getEnclosingClass() === field.getEnclosingClass() &&
+        value.hasGuaranteedType()) {
+      currentFieldSetters[field] = type;
     }
   }
 
-  void handleFieldGet(HFieldGet field, HType type) {
-    assert(compiler.phase == Compiler.PHASE_RECOMPILING);
-    if (!type.isConflicting()) {
-      // If there are no invoked setters with this name, the union of
-      // the types of the initializers and the setters is guaranteed
-      // otherwise it is only speculative.
-      Element element = field.element;
-      assert(!element.isGenerativeConstructorBody());
-      if (!compiler.codegenWorld.hasInvokedSetter(element, compiler)) {
-        field.guaranteedType =
-            type.union(backend.fieldSettersTypeSoFar(element));
-      } else {
-        types[field] = type.union(backend.fieldSettersTypeSoFar(element));
-      }
-    }
-  }
+  visitExit(HExit node) {
+    // If this has been exposed then we cannot say anything about types after
+    // construction.
+    if (thisExposed) return;
 
-  void handleFieldNumberOperation(HFieldGet field, HType type) {
-    assert(compiler.phase == Compiler.PHASE_RECOMPILING);
-    if (compiler.codegenWorld.hasInvokedSetter(field.element, compiler)) {
-      // If there are invoked setters we don't know for sure
-      // that the field will hold a value of the calculated
-      // type, but the fact that the class itself sticks to
-      // this type for the field is still a strong signal
-      // indicating the expected type of the field.
-      types[field] = type;
-    } else {
-      // If there are no invoked setters we know the type of
-      // this field for sure.
-      field.guaranteedType = type;
-    }
+    // Register the known field types.
+    currentFieldSetters.forEach((Element element, HType type) {
+      backend.registerFieldConstructor(element, type);
+      allSetters.remove(element);
+    });
+    allSetters.forEach((Element element) {
+      backend.registerFieldConstructor(element, HType.UNKNOWN);

[ngeoffray, 2012/09/24 15:39:29]

So this is just to prevent thinking a field has this type because of an
initializer? Please add a comment.

[Søren Gjesse, 2012/09/25 13:33:21]

Done.

[Søren Gjesse, 2012/09/25 13:33:21]

Done.

+    });
   }
 }