Improve load forwarding:

runtime/vm/flow_graph_optimizer.cc

Index: runtime/vm/flow_graph_optimizer.cc
diff --git a/runtime/vm/flow_graph_optimizer.cc b/runtime/vm/flow_graph_optimizer.cc
index 15847b4836cc186a5f429b06b47a5ebad49017d4..14d37c37e8d81952d3c088c45c73c4e491a94e2a 100644
--- a/runtime/vm/flow_graph_optimizer.cc
+++ b/runtime/vm/flow_graph_optimizer.cc
@@ -302,7 +302,8 @@ static void ReplaceCurrentInstruction(ForwardInstructionIterator* iterator,
 }
 
 
-void FlowGraphOptimizer::Canonicalize() {
+bool FlowGraphOptimizer::Canonicalize() {
+  bool changed = false;
   for (intptr_t i = 0; i < block_order_.length(); ++i) {
     BlockEntryInstr* entry = block_order_[i];
     entry->Accept(this);
@@ -314,9 +315,11 @@ void FlowGraphOptimizer::Canonicalize() {
         // this.
         ASSERT((replacement == NULL) || current->IsDefinition());
         ReplaceCurrentInstruction(&it, current, replacement, flow_graph_);
+        changed = true;
       }
     }
   }
+  return changed;
 }
 
 
@@ -1275,12 +1278,12 @@ void FlowGraphOptimizer::InlineImplicitInstanceGetter(InstanceCallInstr* call) {
       field.Offset(),
       AbstractType::ZoneHandle(field.type()),
       field.is_final());
+  Field* the_field = &Field::ZoneHandle(field.raw());
+  load->set_field(the_field);
   if (field.guarded_cid() != kIllegalCid) {
     if (!field.is_nullable() || (field.guarded_cid() == kNullCid)) {
       load->set_result_cid(field.guarded_cid());
     }
-    Field* the_field = &Field::ZoneHandle(field.raw());
-    load->set_field(the_field);
     AddToGuardedFields(the_field);
   }
   load->set_field_name(String::Handle(field.name()).ToCString());
@@ -3290,22 +3293,23 @@ class Alias : public ValueObject {
     return Alias(kIndexesAlias);
   }
 
-  // Field load/stores alias each other when field offset matches.
-  // TODO(vegorov): use field information to disambiguate load/stores into
-  // different fields that by accident share offset.
-  static Alias Field(intptr_t offset_in_bytes) {
+  // Field load/stores alias each other only when they access the same field.
+  // AliasedSet assigns ids to a combination of instance and field during
+  // the optimization phase.
+  static Alias Field(intptr_t id) {
+    ASSERT(id >= kFirstFieldAlias);
+    return Alias(id * 2 + 1);
+  }
+
+  // VMField load/stores alias each other when field offset matches.
+  // TODO(vegorov) storing a context variable does not alias loading array
+  // length.
+  static Alias VMField(intptr_t offset_in_bytes) {
     const intptr_t idx = offset_in_bytes / kWordSize;
     ASSERT(idx >= kFirstFieldAlias);
     return Alias(idx * 2);
   }
 
-  // Static field load/stores alias each other.
-  // AliasedSet assigns ids to static fields during optimization phase.
-  static Alias StaticField(intptr_t id) {
-    ASSERT(id >= kFirstFieldAlias);
-    return Alias(id * 2 + 1);
-  }
-
   // Current context load/stores alias each other.
   static Alias CurrentContext() {
     return Alias(kCurrentContextAlias);
@@ -3358,7 +3362,12 @@ class AliasedSet : public ZoneAllocated {
 
     LoadFieldInstr* load_field = defn->AsLoadField();
     if (load_field != NULL) {
-      return Alias::Field(load_field->offset_in_bytes());
+      if (load_field->field() != NULL) {
+        Definition* instance = load_field->instance()->definition();
+        return Alias::Field(GetInstanceFieldId(instance, *load_field->field()));
+      } else {
+        return Alias::VMField(load_field->offset_in_bytes());
+      }
     }
 
     if (defn->IsCurrentContext()) {
@@ -3367,7 +3376,7 @@ class AliasedSet : public ZoneAllocated {
 
     LoadStaticFieldInstr* load_static_field = defn->AsLoadStaticField();
     if (load_static_field != NULL) {
-      return Alias::StaticField(GetFieldId(load_static_field->field()));
+      return Alias::Field(GetFieldId(kAnyInstance, load_static_field->field()));
     }
 
     UNREACHABLE();
@@ -3382,12 +3391,14 @@ class AliasedSet : public ZoneAllocated {
     StoreInstanceFieldInstr* store_instance_field =
         instr->AsStoreInstanceField();
     if (store_instance_field != NULL) {
-      return Alias::Field(store_instance_field->field().Offset());
+      Definition* instance = store_instance_field->instance()->definition();
+      return Alias::Field(GetInstanceFieldId(instance,
+                                             store_instance_field->field()));
     }
 
     StoreVMFieldInstr* store_vm_field = instr->AsStoreVMField();
     if (store_vm_field != NULL) {
-      return Alias::Field(store_vm_field->offset_in_bytes());
+      return Alias::VMField(store_vm_field->offset_in_bytes());
     }
 
     if (instr->IsStoreContext() || instr->IsChainContext()) {
@@ -3396,7 +3407,7 @@ class AliasedSet : public ZoneAllocated {
 
     StoreStaticFieldInstr* store_static_field = instr->AsStoreStaticField();
     if (store_static_field != NULL) {
-      return Alias::StaticField(GetFieldId(store_static_field->field()));
+      return Alias::Field(GetStaticFieldId(store_static_field->field()));
     }
 
     return Alias::None();
@@ -3438,18 +3449,68 @@ class AliasedSet : public ZoneAllocated {
 
   // Get id assigned to the given field. Assign a new id if the field is seen
   // for the first time.
-  intptr_t GetFieldId(const Field& field) {
-    intptr_t id = field_ids_.Lookup(&field);
+  intptr_t GetFieldId(intptr_t instance_id, const Field& field) {
+    intptr_t id = field_ids_.Lookup(FieldIdPair::Key(instance_id, &field));
     if (id == 0) {
       id = ++max_field_id_;
-      field_ids_.Insert(FieldIdPair(&field, id));
+      field_ids_.Insert(FieldIdPair(FieldIdPair::Key(instance_id, &field), id));
     }
     return id;
   }
 
+  enum {
+    kAnyInstance = -1
+  };
+
+  intptr_t GetInstanceFieldId(Definition* defn, const Field& field) {
+    intptr_t instance_id = kAnyInstance;
+
+    AllocateObjectInstr* alloc = defn->AsAllocateObject();
+    if ((alloc != NULL) && !CanBeAliased(alloc)) {
+      instance_id = alloc->ssa_temp_index();
+      ASSERT(instance_id != kAnyInstance);
+    }
+
+    return GetFieldId(instance_id, field);
+  }
+
+  intptr_t GetStaticFieldId(const Field& field) {
+    return GetFieldId(kAnyInstance, field);
+  }
+
+  intptr_t CanBeAliased(AllocateObjectInstr* alloc) {
+    if (alloc->identity() == AllocateObjectInstr::kUnknown) {
+      bool escapes = false;
+      for (Value* use = alloc->input_use_list();
+           use != NULL;
+           use = use->next_use()) {
+        Instruction* instr = use->instruction();
+        if (instr->IsPushArgument() ||
+            (instr->IsStoreVMField() && use->use_index() != 0) ||
+            (instr->IsStoreInstanceField() && use->use_index() != 0) ||

[srdjan, 2013/05/02 21:00:54]

Some more parentheses, please.

[Vyacheslav Egorov (Google), 2013/05/02 21:20:25]

Done.

+            (instr->IsStoreStaticField()) ||
+            (instr->IsPhi())) {
+          escapes = true;
+          break;
+        }
+
+        alloc->set_identity(escapes ? AllocateObjectInstr::kAliased
+                                    : AllocateObjectInstr::kNotAliased);
+      }
+    }
+    return alloc->identity() != AllocateObjectInstr::kNotAliased;
+  }
+
   class FieldIdPair {
    public:
-    typedef const Field* Key;
+    struct Key {
+      Key(intptr_t instance_id, const Field* field)
+          : instance_id_(instance_id), field_(field) { }
+
+      intptr_t instance_id_;
+      const Field* field_;
+    };
+
     typedef intptr_t Value;
     typedef FieldIdPair Pair;
 
@@ -3464,11 +3525,12 @@ class AliasedSet : public ZoneAllocated {
     }
 
     static intptr_t Hashcode(Key key) {
-      return String::Handle(key->name()).Hash();
+      return String::Handle(key.field_->name()).Hash();
     }
 
     static inline bool IsKeyEqual(Pair kv, Key key) {
-      return KeyOf(kv)->raw() == key->raw();
+      return (KeyOf(kv).field_->raw() == key.field_->raw()) &&
+          (KeyOf(kv).instance_id_ == key.instance_id_);
     }
 
    private:
@@ -3541,7 +3603,7 @@ class LoadKeyValueTrait {
       location = store_indexed->index()->definition()->ssa_temp_index();
     } else if (key->IsLoadField()) {
       LoadFieldInstr* load_field = key->AsLoadField();
-      object = load_field->value()->definition()->ssa_temp_index();
+      object = load_field->instance()->definition()->ssa_temp_index();
       location = load_field->offset_in_bytes();
     } else if (key->IsStoreInstanceField()) {
       StoreInstanceFieldInstr* store_field = key->AsStoreInstanceField();
@@ -3597,11 +3659,11 @@ class LoadKeyValueTrait {
     LoadFieldInstr* load_field = kv->AsLoadField();
     if (key->IsStoreVMField()) {
       StoreVMFieldInstr* store_field = key->AsStoreVMField();
-      return load_field->value()->Equals(store_field->dest()) &&
+      return load_field->instance()->Equals(store_field->dest()) &&
              (load_field->offset_in_bytes() == store_field->offset_in_bytes());
     } else if (key->IsStoreInstanceField()) {
       StoreInstanceFieldInstr* store_field = key->AsStoreInstanceField();
-      return load_field->value()->Equals(store_field->instance()) &&
+      return load_field->instance()->Equals(store_field->instance()) &&
              (load_field->offset_in_bytes() == store_field->field().Offset());
     }
 
@@ -4727,16 +4789,16 @@ void ConstantPropagator::VisitLoadUntagged(LoadUntaggedInstr* instr) {
 
 void ConstantPropagator::VisitLoadField(LoadFieldInstr* instr) {
   if ((instr->recognized_kind() == MethodRecognizer::kObjectArrayLength) &&
-      (instr->value()->definition()->IsCreateArray())) {
+      (instr->instance()->definition()->IsCreateArray())) {
     const intptr_t length =
-        instr->value()->definition()->AsCreateArray()->num_elements();
+        instr->instance()->definition()->AsCreateArray()->num_elements();
     const Object& result = Smi::ZoneHandle(Smi::New(length));
     SetValue(instr, result);
     return;
   }
 
   if (instr->IsImmutableLengthLoad()) {
-    ConstantInstr* constant = instr->value()->definition()->AsConstant();
+    ConstantInstr* constant = instr->instance()->definition()->AsConstant();
     if (constant != NULL) {
       if (constant->value().IsString()) {
         SetValue(instr, Smi::ZoneHandle(