Create and use new change/depends flags for the GVN analysis.

lib/compiler/implementation/ssa/nodes.dart

Index: lib/compiler/implementation/ssa/nodes.dart
===================================================================
--- lib/compiler/implementation/ssa/nodes.dart	(revision 13049)
+++ lib/compiler/implementation/ssa/nodes.dart	(working copy)
@@ -757,11 +757,18 @@
   int flags = 0;
 
   // Changes flags.
-  static const int FLAG_CHANGES_SOMETHING    = 0;
-  static const int FLAG_CHANGES_COUNT        = FLAG_CHANGES_SOMETHING + 1;
+  static const int FLAG_CHANGES_INDEX             = 0;
+  static const int FLAG_CHANGES_PROPERTY          = FLAG_CHANGES_INDEX + 1;
+  static const int FLAG_CHANGES_STATIC            = FLAG_CHANGES_PROPERTY + 1;

[Søren Gjesse, 2012/10/01 07:22:20]

How about FLAG_CHANGES_STATIC_PROPERTY (and then maybe
FLAG_CHANGES_OBJECT_PROPERTY above)? - or will that be too long?

[ngeoffray, 2012/10/04 10:41:42]

Done.

+  static const int FLAG_CHANGES_SOMETHING         = FLAG_CHANGES_STATIC + 1;
+  static const int FLAG_CHANGES_COUNT             = FLAG_CHANGES_SOMETHING + 1;
 
   // Depends flags (one for each changes flag).
-  static const int FLAG_DEPENDS_ON_SOMETHING = FLAG_CHANGES_COUNT;
+  static const int FLAG_DEPENDS_ON_INDEX_STORE = FLAG_CHANGES_COUNT;
+  static const int FLAG_DEPENDS_ON_PROPERTY_STORE =
+      FLAG_DEPENDS_ON_INDEX_STORE + 1;
+  static const int FLAG_DEPENDS_ON_STATIC = FLAG_DEPENDS_ON_PROPERTY_STORE + 1;
+  static const int FLAG_DEPENDS_ON_SOMETHING = FLAG_DEPENDS_ON_STATIC + 1;
 
   // Other flags.
   static const int FLAG_USE_GVN              = FLAG_DEPENDS_ON_SOMETHING + 1;
@@ -800,6 +807,7 @@
   static const int BAILOUT_TARGET_TYPECODE = 29;
   static const int INVOKE_STATIC_TYPECODE = 30;
   static const int INVOKE_DYNAMIC_GETTER_TYPECODE = 31;
+  static const int INDEX_TYPECODE = 32;
 
   HInstruction(this.inputs)
       : id = idCounter++,
@@ -823,6 +831,18 @@
   bool dependsOnSomething() => getFlag(FLAG_DEPENDS_ON_SOMETHING);
   void setDependsOnSomething() { setFlag(FLAG_DEPENDS_ON_SOMETHING); }
 
+  bool dependsOnStatic() => getFlag(FLAG_DEPENDS_ON_STATIC);

[Søren Gjesse, 2012/10/01 07:22:20]

dependsOnStaticPropertyStore?

[ngeoffray, 2012/10/04 10:41:42]

Done.

+  void setDependsOnStatic() { setFlag(FLAG_DEPENDS_ON_STATIC); }
+  void setChangesStatic() { setFlag(FLAG_CHANGES_STATIC); }
+
+  bool dependsOnIndexStore() => getFlag(FLAG_DEPENDS_ON_INDEX_STORE);
+  void setDependsOnIndexStore() { setFlag(FLAG_DEPENDS_ON_INDEX_STORE); }
+  void setChangesIndex() { setFlag(FLAG_CHANGES_INDEX); }
+
+  bool dependsOnPropertyStore() => getFlag(FLAG_DEPENDS_ON_PROPERTY_STORE);
+  void setDependsOnPropertyStore() { setFlag(FLAG_DEPENDS_ON_PROPERTY_STORE); }
+  void setChangesProperty() { setFlag(FLAG_CHANGES_PROPERTY); }
+
   bool useGvn() => getFlag(FLAG_USE_GVN);
   void setUseGvn() { setFlag(FLAG_USE_GVN); }
   // Does this node potentially affect control flow.
@@ -1298,8 +1318,10 @@
 }
 
 class HInvokeDynamicField extends HInvokeDynamic {
-  HInvokeDynamicField(Selector selector, Element element,
-                      List<HInstruction> inputs)
+  final bool isSideEffectFree;
+  HInvokeDynamicField(
+      Selector selector, Element element, List<HInstruction> inputs,
+      this.isSideEffectFree)
       : super(selector, element, inputs);
   toString() => 'invoke dynamic field: $selector';
 
@@ -1308,10 +1330,9 @@
 }
 
 class HInvokeDynamicGetter extends HInvokeDynamicField {
-  final bool isSideEffectFree;
   HInvokeDynamicGetter(
-      selector, element, receiver, this.isSideEffectFree)
-    : super(selector, element,[receiver]);
+      selector, element, receiver, isSideEffectFree)
+    : super(selector, element, [receiver], isSideEffectFree);
   toString() => 'invoke dynamic getter: $selector';
   accept(HVisitor visitor) => visitor.visitInvokeDynamicGetter(this);
 
@@ -1319,7 +1340,7 @@
     if (isSideEffectFree) {
       setUseGvn();
       clearAllSideEffects();
-      setDependsOnSomething();
+      setDependsOnPropertyStore();
     } else {
       setAllSideEffects();
     }
@@ -1331,10 +1352,18 @@
 }
 
 class HInvokeDynamicSetter extends HInvokeDynamicField {
-  HInvokeDynamicSetter(selector, element, receiver, value)
-    : super(selector, element, [receiver, value]);
+  HInvokeDynamicSetter(selector, element, receiver, value, isSideEffectFree)
+    : super(selector, element, [receiver, value], isSideEffectFree);
   toString() => 'invoke dynamic setter: $selector';
   accept(HVisitor visitor) => visitor.visitInvokeDynamicSetter(this);
+
+  void prepareGvn(HTypeMap types) {
+    if (isSideEffectFree) {
+      setChangesProperty();
+    } else {
+      setAllSideEffects();
+    }
+  }
 }
 
 class HInvokeStatic extends HInvoke {
@@ -1436,11 +1465,16 @@
       // We cannot do the same thing for non-extendable array because
       // we don't express that type yet: a mutable array might be
       // extendable.
-      if (!inputs[1].isString(types)) setDependsOnSomething();
+      if (!inputs[1].isString(types)) setDependsOnPropertyStore();
     } else if (isSideEffectFree) {
       setUseGvn();
       clearAllSideEffects();
       setDependsOnSomething();
+    } else if (selector.isGetter()) {
+      // Getter interceptors do not have side effects.

[floitsch, 2012/10/01 08:40:30]

I don't think this is true.
o.length is an HInvokeInterceptor, but you can't be sure that it won't have a
side-effect.

[ngeoffray, 2012/10/01 21:56:10]

Right, I was only thinking about getters on primitive types. Code removed.

+      setUseGvn();
+      clearAllSideEffects();
+      setDependsOnPropertyStore();
     } else {
       setAllSideEffects();
     }
@@ -1475,7 +1509,9 @@
   void prepareGvn(HTypeMap types) {
     setUseGvn();
     clearAllSideEffects();
-    if (isAssignable) setDependsOnSomething();
+    if (isAssignable) {
+      setDependsOnPropertyStore();
+    }
   }
 
   int typeCode() => HInstruction.FIELD_GET_TYPECODE;
@@ -1495,8 +1531,7 @@
   accept(HVisitor visitor) => visitor.visitFieldSet(this);
 
   void prepareGvn(HTypeMap types) {
-    // TODO(ngeoffray): implement more fine grained side effects.
-    setAllSideEffects();
+    setChangesProperty();
   }
 
   bool isJsStatement(HTypeMap types) => true;
@@ -1509,14 +1544,6 @@
   accept(HVisitor visitor) => visitor.visitLocalGet(this);
 
   HLocalValue get local => inputs[0];
-
-  void prepareGvn(HTypeMap types) {
-    setUseGvn();
-    // TODO(floitsch): if the variable is not captured then it only depends
-    // on assignments to the same variable. Otherwise we need to see if the
-    // variable is mutated inside closures.
-    setDependsOnSomething();
-  }
 }
 
 class HLocalSet extends HFieldSet {
@@ -1526,11 +1553,6 @@
   accept(HVisitor visitor) => visitor.visitLocalSet(this);
 
   HLocalValue get local => inputs[0];
-
-  void prepareGvn(HTypeMap types) {
-    // TODO(floitsch): implement more fine grained side effects.
-    setAllSideEffects();
-  }
 }
 
 class HForeign extends HInstruction {
@@ -2378,8 +2400,10 @@
   void prepareGvn(HTypeMap types) {
     if (!element.isAssignable()) {
       clearAllSideEffects();
-      setUseGvn();
+    } else {
+      setDependsOnStatic();
     }
+    setUseGvn();
   }
   toString() => 'static ${element.name}';
   accept(HVisitor visitor) => visitor.visitStatic(this);
@@ -2419,6 +2443,10 @@
   bool typeEquals(other) => other is HStaticStore;
   bool dataEquals(HStaticStore other) => element == other.element;
   bool isJsStatement(HTypeMap types) => true;
+
+  void prepareGvn(HTypeMap types) {
+    setChangesStatic();
+  }
 }
 
 class HLiteralList extends HInstruction {
@@ -2442,6 +2470,8 @@
   void prepareGvn(HTypeMap types) {
     if (isBuiltin(types)) {
       clearAllSideEffects();
+      setDependsOnIndexStore();
+      setUseGvn();
     } else {
       setAllSideEffects();
     }
@@ -2465,6 +2495,10 @@
 
   bool isBuiltin(HTypeMap types)
       => receiver.isIndexablePrimitive(types) && index.isInteger(types);
+
+  int typeCode() => HInstruction.INDEX_TYPECODE;
+  bool typeEquals(HInstruction other) => other is HIndex;
+  bool dataEquals(HIndex other) => true;
 }
 
 class HIndexAssign extends HInvokeStatic {
@@ -2480,6 +2514,14 @@
   HInstruction get index => inputs[2];
   HInstruction get value => inputs[3];
 
+  void prepareGvn(HTypeMap types) {
+    if (isBuiltin(types)) {
+      setChangesIndex();
+    } else {
+      setAllSideEffects();
+    }
+  }
+
   // Note, that we don't have a computeTypeFromInputTypes, since [HIndexAssign]
   // is never used as input.