Track side-effect free paths in the graph to allow CSE and LICM for instructions that depend on som…

runtime/vm/intermediate_language.cc

Index: runtime/vm/intermediate_language.cc
diff --git a/runtime/vm/intermediate_language.cc b/runtime/vm/intermediate_language.cc
index ea389c7cebcfbc8bf73e848bbecec5a335077668..2ef374785c66676840af54c72d545771199b39dd 100644
--- a/runtime/vm/intermediate_language.cc
+++ b/runtime/vm/intermediate_language.cc
@@ -103,11 +103,12 @@ bool CheckClassInstr::AttributesEqual(Instruction* other) const {
 }
 
 
-bool CheckClassInstr::AffectedBySideEffect() const {
-  // The class-id of string objects is not invariant: Externalization of strings
-  // via the API can change the class-id.
-  return unary_checks().HasReceiverClassId(kOneByteStringCid)
-      || unary_checks().HasReceiverClassId(kTwoByteStringCid);
+EffectSet CheckClassInstr::Dependencies() const {
+  // Externalization of strings via the API can change the class-id.
+  const bool externalizable =
+      unary_checks().HasReceiverClassId(kOneByteStringCid) ||
+      unary_checks().HasReceiverClassId(kTwoByteStringCid);

[srdjan, 2013/04/29 17:16:26]

Need to add all typed data arrays as well.

[Vyacheslav Egorov (Google), 2013/04/30 14:01:33]

I suggest we add them in a separate CL.

+  return externalizable ? EffectSet::All() : EffectSet::None();

[Florian Schneider, 2013/04/29 12:11:54]

Shouldn't the dependencies be EffectSet::kExternalizable?

Right now, there is no difference because it is equals to All().

[Vyacheslav Egorov (Google), 2013/04/30 14:01:33]

Done.

 }
 
 
@@ -116,11 +117,6 @@ bool GuardFieldInstr::AttributesEqual(Instruction* other) const {
 }
 
 
-bool GuardFieldInstr::AffectedBySideEffect() const {
-  return false;
-}
-
-
 bool CheckArrayBoundInstr::AttributesEqual(Instruction* other) const {
   CheckArrayBoundInstr* other_check = other->AsCheckArrayBound();
   ASSERT(other_check != NULL);
@@ -153,6 +149,11 @@ bool BinarySmiOpInstr::AttributesEqual(Instruction* other) const {
 }
 
 
+EffectSet LoadFieldInstr::Dependencies() const {
+  return immutable_ ? EffectSet::None() : EffectSet::All();
+}
+
+
 bool LoadFieldInstr::AttributesEqual(Instruction* other) const {
   LoadFieldInstr* other_load = other->AsLoadField();
   ASSERT(other_load != NULL);
@@ -162,6 +163,11 @@ bool LoadFieldInstr::AttributesEqual(Instruction* other) const {
 }
 
 
+EffectSet LoadStaticFieldInstr::Dependencies() const {
+  return field().is_final() ? EffectSet::None() : EffectSet::All();
+}
+
+
 bool LoadStaticFieldInstr::AttributesEqual(Instruction* other) const {
   LoadStaticFieldInstr* other_load = other->AsLoadStaticField();
   ASSERT(other_load != NULL);
@@ -172,6 +178,11 @@ bool LoadStaticFieldInstr::AttributesEqual(Instruction* other) const {
 }
 
 
+EffectSet LoadIndexedInstr::Dependencies() const {
+  return EffectSet::All();
+}
+
+
 bool LoadIndexedInstr::AttributesEqual(Instruction* other) const {
   LoadIndexedInstr* other_load = other->AsLoadIndexed();
   ASSERT(other_load != NULL);
@@ -1710,8 +1721,10 @@ static bool AreEqualDefinitions(Definition* a, Definition* b) {
   a = UnwrapConstraint(a);
   b = UnwrapConstraint(b);
   return (a == b) ||
-      (!a->AffectedBySideEffect() &&
-       !b->AffectedBySideEffect() &&
+      (a->AllowsCSE() &&
+       a->Dependencies().IsNone() &&
+       b->AllowsCSE() &&
+       b->Dependencies().IsNone() &&
        a->Equals(b));
 }