Bug fix in load elimination, constant propagation for LoadClassId.

runtime/vm/flow_graph_optimizer.cc

 // Copyright (c) 2013, 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.
 
 #include "vm/flow_graph_optimizer.h"
 
 #include "vm/bit_vector.h"
 #include "vm/cha.h"
 #include "vm/dart_entry.h"
 #include "vm/flow_graph_builder.h"
@@ skipping 4130 matching lines @@
         id_(other.id_) {
   }
 
   // Construct a place from instruction if instruction accesses any place.
   // Otherwise constructs kNone place.
   Place(Instruction* instr, bool* is_load)
       : kind_(kNone), instance_(NULL), raw_selector_(0), id_(0) {
     switch (instr->tag()) {
       case Instruction::kLoadField: {
         LoadFieldInstr* load_field = instr->AsLoadField();
-        instance_ = load_field->instance()->definition();
+        instance_ = Unwrap(load_field->instance()->definition());
         if (load_field->field() != NULL) {
           kind_ = kField;
           field_ = load_field->field();
         } else {
           kind_ = kVMField;
           offset_in_bytes_ = load_field->offset_in_bytes();
         }
         *is_load = true;
         break;
       }
 
       case Instruction::kStoreInstanceField: {
         StoreInstanceFieldInstr* store_instance_field =
             instr->AsStoreInstanceField();
         kind_ = kField;
-        instance_ = store_instance_field->instance()->definition();
+        instance_ = Unwrap(store_instance_field->instance()->definition());
         field_ = &store_instance_field->field();
         break;
       }
 
       case Instruction::kStoreVMField: {
         StoreVMFieldInstr* store_vm_field = instr->AsStoreVMField();
         kind_ = kVMField;
-        instance_ = store_vm_field->dest()->definition();
+        instance_ = Unwrap(store_vm_field->dest()->definition());
         offset_in_bytes_ = store_vm_field->offset_in_bytes();
         break;
       }
 
       case Instruction::kLoadStaticField:
         kind_ = kField;
         field_ = &instr->AsLoadStaticField()->StaticField();
         *is_load = true;
         break;
 
       case Instruction::kStoreStaticField:
         kind_ = kField;
         field_ = &instr->AsStoreStaticField()->field();
         break;
 
       case Instruction::kLoadIndexed: {
         LoadIndexedInstr* load_indexed = instr->AsLoadIndexed();
         kind_ = kIndexed;
-        instance_ = load_indexed->array()->definition();
+        instance_ = Unwrap(load_indexed->array()->definition());
         index_ = load_indexed->index()->definition();
         *is_load = true;
         break;
       }
 
       case Instruction::kStoreIndexed: {
         StoreIndexedInstr* store_indexed = instr->AsStoreIndexed();
         kind_ = kIndexed;
-        instance_ = store_indexed->array()->definition();
+        instance_ = Unwrap(store_indexed->array()->definition());
         index_ = store_indexed->index()->definition();
         break;
       }
 
       case Instruction::kCurrentContext:
         kind_ = kContext;
         *is_load = true;
         break;
 
       case Instruction::kStoreContext:
@@ skipping 10 matching lines @@
 
   Kind kind() const { return kind_; }
 
   Definition* instance() const {
     ASSERT((kind_ == kField) || (kind_ == kVMField) || (kind_ == kIndexed));
     return instance_;
   }
 
   void set_instance(Definition* def) {
     ASSERT((kind_ == kField) || (kind_ == kVMField) || (kind_ == kIndexed));
-    instance_ = def;
+    instance_ = Unwrap(def);
   }
 
   const Field& field() const {
     ASSERT(kind_ == kField);
     return *field_;
   }
 
   intptr_t offset_in_bytes() const {
     ASSERT(kind_ == kVMField);
     return offset_in_bytes_;
@@ skipping 50 matching lines @@
   bool Equals(Place* other) const {
     return (kind_ == other->kind_) &&
         (instance_ == other->instance_) &&
         SameField(other);
   }
 
   // Create a zone allocated copy of this place.
   static Place* Wrap(const Place& place);
 
  private:
+  static Definition* Unwrap(Definition* defn) {

[Kevin Millikin (Google), 2013/10/02 12:36:19]

Even though it's only used locally, this name is too generic.  Try something
snazzy like OriginalDefinitionOf or RealDefinitionOf.

[Florian Schneider, 2013/10/02 12:58:41]

Done.

+    while (defn->IsRedefinition()) {
+      defn = defn->AsRedefinition()->value()->definition();
+    }
+    return defn;
+  }
+
   bool SameField(Place* other) const {
     return (kind_ == kField) ? (field().raw() == other->field().raw())
                              : (offset_in_bytes_ == other->offset_in_bytes_);
   }
 
   intptr_t FieldHashcode() const {
     return (kind_ == kField) ? reinterpret_cast<intptr_t>(field().raw())
                              : offset_in_bytes_;
   }
 
@@ skipping 1958 matching lines @@
   SetValue(instr, non_constant_);
 }
 
 
 void ConstantPropagator::VisitLoadUntagged(LoadUntaggedInstr* instr) {
   SetValue(instr, non_constant_);
 }
 
 
 void ConstantPropagator::VisitLoadClassId(LoadClassIdInstr* instr) {
+  intptr_t cid = instr->object()->Type()->ToCid();
+  if (cid != kDynamicCid) {
+    SetValue(instr, Smi::ZoneHandle(Smi::New(cid)));
+    return;
+  }
   SetValue(instr, non_constant_);

[Kevin Millikin (Google), 2013/10/02 12:36:19]

You could also try this: if instr->object() has a constant value that is an
instance (I guess it should be), then set this instruction's constant value to
that one's class ID.

[Florian Schneider, 2013/10/02 12:58:41]

Done.

 }
 
 
 void ConstantPropagator::VisitLoadField(LoadFieldInstr* instr) {
   if ((instr->recognized_kind() == MethodRecognizer::kObjectArrayLength) &&
       (instr->instance()->definition()->IsCreateArray())) {
     const intptr_t length =
         instr->instance()->definition()->AsCreateArray()->num_elements();
     const Object& result = Smi::ZoneHandle(Smi::New(length));
     SetValue(instr, result);
@@ skipping 1341 matching lines @@
   }
 
   // Insert materializations at environment uses.
   for (intptr_t i = 0; i < exits.length(); i++) {
     CreateMaterializationAt(exits[i], alloc, alloc->cls(), *fields);
   }
 }
 
 
 }  // namespace dart