Convert some compiler passes to preserve valid def-use chains.

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/flow_graph_builder.h"
 #include "vm/flow_graph_compiler.h"
@@ skipping 31 matching lines @@
 
 // Attempts to convert an instance call (IC call) using propagated class-ids,
 // e.g., receiver class id.
 void FlowGraphOptimizer::ApplyClassIds() {
   ASSERT(current_iterator_ == NULL);
   for (intptr_t i = 0; i < block_order_.length(); ++i) {
     BlockEntryInstr* entry = block_order_[i];
     ForwardInstructionIterator it(entry);
     current_iterator_ = &it;
     for (; !it.Done(); it.Advance()) {
-      if (it.Current()->IsInstanceCall()) {
-        InstanceCallInstr* call = it.Current()->AsInstanceCall();
+      Instruction* instr = it.Current();
+      if (instr->IsInstanceCall()) {
+        InstanceCallInstr* call = instr->AsInstanceCall();
         if (call->HasICData()) {
           if (TryCreateICData(call)) {
             VisitInstanceCall(call);
           }
         }
-      } else if (it.Current()->IsPolymorphicInstanceCall()) {
-        SpecializePolymorphicInstanceCall(
-            it.Current()->AsPolymorphicInstanceCall());
-      } else if (it.Current()->IsStrictCompare()) {
-        VisitStrictCompare(it.Current()->AsStrictCompare());
-      } else if (it.Current()->IsBranch()) {
-        ComparisonInstr* compare = it.Current()->AsBranch()->comparison();
+      } else if (instr->IsPolymorphicInstanceCall()) {
+        SpecializePolymorphicInstanceCall(instr->AsPolymorphicInstanceCall());
+      } else if (instr->IsStrictCompare()) {
+        VisitStrictCompare(instr->AsStrictCompare());
+      } else if (instr->IsBranch()) {
+        ComparisonInstr* compare = instr->AsBranch()->comparison();
         if (compare->IsStrictCompare()) {
           VisitStrictCompare(compare->AsStrictCompare());
         }
       }
     }
     current_iterator_ = NULL;
   }
 }
 
 
 // Attempt to build ICData for call using propagated class-ids.
 bool FlowGraphOptimizer::TryCreateICData(InstanceCallInstr* call) {
   ASSERT(call->HasICData());
   if (call->ic_data()->NumberOfChecks() > 0) {
     // This occurs when an instance call has too many checks.
     // TODO(srdjan): Replace IC call with megamorphic call.
     return false;
   }
   GrowableArray<intptr_t> class_ids(call->ic_data()->num_args_tested());
   ASSERT(call->ic_data()->num_args_tested() <= call->ArgumentCount());
   for (intptr_t i = 0; i < call->ic_data()->num_args_tested(); i++) {
-    intptr_t cid = call->ArgumentAt(i)->value()->ResultCid();
+    intptr_t cid = call->PushArgumentAt(i)->value()->ResultCid();
     class_ids.Add(cid);
   }
   // TODO(srdjan): Test for other class_ids > 1.
   if (class_ids.length() != 1) return false;
   if (class_ids[0] != kDynamicCid) {
     const intptr_t num_named_arguments = call->argument_names().IsNull() ?
         0 : call->argument_names().Length();
     const Class& receiver_class = Class::Handle(
         Isolate::Current()->class_table()->At(class_ids[0]));
     Function& function = Function::Handle();
@@ skipping 45 matching lines @@
   return new_ic_data;
 }
 
 
 void FlowGraphOptimizer::SpecializePolymorphicInstanceCall(
     PolymorphicInstanceCallInstr* call) {
   if (!call->with_checks()) {
     return;  // Already specialized.
   }
 
-  const intptr_t receiver_cid  = call->ArgumentAt(0)->value()->ResultCid();
+  const intptr_t receiver_cid = call->PushArgumentAt(0)->value()->ResultCid();
   if (receiver_cid == kDynamicCid) {
     return;  // No information about receiver was infered.
   }
 
   const ICData& ic_data = SpecializeICData(call->ic_data(), receiver_cid);
 
   const bool with_checks = false;
   PolymorphicInstanceCallInstr* specialized =
       new PolymorphicInstanceCallInstr(call->instance_call(),
                                        ic_data,
@@ skipping 63 matching lines @@
                                           Representation to,
                                           Value* use,
                                           Instruction* insert_before,
                                           Instruction* deopt_target) {
   Definition* converted = NULL;
   if ((from == kTagged) && (to == kUnboxedMint)) {
     ASSERT((deopt_target != NULL) ||
            (use->definition()->GetPropagatedCid() == kDoubleCid));
     const intptr_t deopt_id = (deopt_target != NULL) ?
         deopt_target->DeoptimizationTarget() : Isolate::kNoDeoptId;
-    converted = new UnboxIntegerInstr(new Value(use->definition()), deopt_id);
+    converted = new UnboxIntegerInstr(use->Copy(), deopt_id);
+
   } else if ((from == kUnboxedMint) && (to == kTagged)) {
-    converted = new BoxIntegerInstr(new Value(use->definition()));
+    converted = new BoxIntegerInstr(use->Copy());
+
   } else if (from == kUnboxedMint && to == kUnboxedDouble) {
     // Convert by boxing/unboxing.
     // TODO(fschneider): Implement direct unboxed mint-to-double conversion.
-    BoxIntegerInstr* boxed = new BoxIntegerInstr(new Value(use->definition()));
+    BoxIntegerInstr* boxed = new BoxIntegerInstr(use->Copy());
+    use->RemoveFromUseList();
+    use->set_definition(boxed);
+    boxed->AddInputUse(use);

[Vyacheslav Egorov (Google), 2013/02/08 16:44:49]

I don't think these three lines are needed. You can remove them.

[Kevin Millikin (Google), 2013/02/19 10:49:30]

You are right.  Thank you.

     InsertBefore(insert_before, boxed, NULL, Definition::kValue);
+
     const intptr_t deopt_id = (deopt_target != NULL) ?
         deopt_target->DeoptimizationTarget() : Isolate::kNoDeoptId;
     converted = new UnboxDoubleInstr(new Value(boxed), deopt_id);
+
   } else if ((from == kUnboxedDouble) && (to == kTagged)) {
-    converted = new BoxDoubleInstr(new Value(use->definition()), NULL);
+    converted = new BoxDoubleInstr(use->Copy(), NULL);
+
   } else if ((from == kTagged) && (to == kUnboxedDouble)) {
+    ASSERT((deopt_target != NULL) ||
+           (use->definition()->GetPropagatedCid() == kDoubleCid));
     const intptr_t deopt_id = (deopt_target != NULL) ?
         deopt_target->DeoptimizationTarget() : Isolate::kNoDeoptId;
-    ASSERT((deopt_target != NULL) ||
-           (use->definition()->GetPropagatedCid() == kDoubleCid));
     ConstantInstr* constant = use->definition()->AsConstant();
     if ((constant != NULL) && constant->value().IsSmi()) {
       const double dbl_val = Smi::Cast(constant->value()).AsDoubleValue();
       const Double& dbl_obj =
           Double::ZoneHandle(Double::New(dbl_val, Heap::kOld));
       ConstantInstr* double_const = new ConstantInstr(dbl_obj);
       InsertBefore(insert_before, double_const, NULL, Definition::kValue);
       converted = new UnboxDoubleInstr(new Value(double_const), deopt_id);
     } else {
-      converted = new UnboxDoubleInstr(new Value(use->definition()), deopt_id);
+      converted = new UnboxDoubleInstr(use->Copy(), deopt_id);
     }
   }
   ASSERT(converted != NULL);
+  use->RemoveFromUseList();
+  use->set_definition(converted);
+  converted->AddInputUse(use);
   InsertBefore(insert_before, converted, use->instruction()->env(),
                Definition::kValue);
-  use->set_definition(converted);
 }
 
 
 void FlowGraphOptimizer::InsertConversionsFor(Definition* def) {
   const Representation from_rep = def->representation();
 
   for (Value::Iterator it(def->input_use_list());
        !it.Done();
        it.Advance()) {
     Value* use = it.Current();
@@ skipping 171 matching lines @@
   }
 
   // Check that it have seen only smis and doubles.
   GrowableArray<intptr_t> class_ids(2);
   class_ids.Add(kSmiCid);
   class_ids.Add(kDoubleCid);
   return ICDataHasOnlyReceiverArgumentClassIds(ic_data, class_ids, class_ids);
 }
 
 
-static void RemovePushArguments(InstanceCallInstr* call) {
-  // Remove original push arguments.
+void FlowGraphOptimizer::ReplaceCall(Definition* call,
+                                     Definition* replacement) {
+  // Remove the original push arguments.
   for (intptr_t i = 0; i < call->ArgumentCount(); ++i) {
-    PushArgumentInstr* push = call->ArgumentAt(i);
+    PushArgumentInstr* push = call->PushArgumentAt(i);
     push->ReplaceUsesWith(push->value()->definition());
+    push->UnuseAllInputs();
     push->RemoveFromGraph();
   }
+  call->ReplaceWith(replacement, current_iterator());
 }
 
 
-static void RemovePushArguments(StaticCallInstr* call) {
-  // Remove original push arguments.
-  for (intptr_t i = 0; i < call->ArgumentCount(); ++i) {
-    PushArgumentInstr* push = call->ArgumentAt(i);
-    push->ReplaceUsesWith(push->value()->definition());
-    push->RemoveFromGraph();
-  }
-}
-
-
 static intptr_t ReceiverClassId(InstanceCallInstr* call) {
   if (!call->HasICData()) return kIllegalCid;
 
   const ICData& ic_data = ICData::Handle(call->ic_data()->AsUnaryClassChecks());
 
   if (ic_data.NumberOfChecks() == 0) return kIllegalCid;
   // TODO(vegorov): Add multiple receiver type support.
   if (ic_data.NumberOfChecks() != 1) return kIllegalCid;
   ASSERT(ic_data.HasOneTarget());
 
   Function& target = Function::Handle();
   intptr_t class_id;
   ic_data.GetOneClassCheckAt(0, &class_id, &target);
   return class_id;
 }
 
 
-void FlowGraphOptimizer::AddCheckClass(InstanceCallInstr* call,
-                                       Value* value) {
+void FlowGraphOptimizer::AddCheckClass(Definition* to_check,
+                                       const ICData& unary_checks,
+                                       intptr_t deopt_id,
+                                       Environment* deopt_environment,
+                                       Instruction* insert_before) {
   // Type propagation has not run yet, we cannot eliminate the check.
-  const ICData& unary_checks =
-      ICData::ZoneHandle(call->ic_data()->AsUnaryClassChecks());
   Instruction* check = NULL;
   if ((unary_checks.NumberOfChecks() == 1) &&
       (unary_checks.GetReceiverClassIdAt(0) == kSmiCid)) {
-    check = new CheckSmiInstr(value, call->deopt_id());
+    check = new CheckSmiInstr(new Value(to_check), deopt_id);
   } else {
-    check = new CheckClassInstr(value, call->deopt_id(), unary_checks);
+    check = new CheckClassInstr(new Value(to_check), deopt_id, unary_checks);
   }
-  InsertBefore(call, check, call->env(), Definition::kEffect);
+  InsertBefore(insert_before, check, deopt_environment, Definition::kEffect);
 }
 
 
+void FlowGraphOptimizer::AddReceiverCheck(InstanceCallInstr* call) {
+  AddCheckClass(call->ArgumentAt(0),
+                ICData::ZoneHandle(call->ic_data()->AsUnaryClassChecks()),
+                call->deopt_id(),
+                call->env(),
+                call);
+}
+
+
 static bool ArgIsAlwaysSmi(const ICData& ic_data, intptr_t arg_n) {
   ASSERT(ic_data.num_args_tested() > arg_n);
   if (ic_data.NumberOfChecks() == 0) return false;
   GrowableArray<intptr_t> class_ids;
   Function& target = Function::Handle();
   const intptr_t len = ic_data.NumberOfChecks();
   for (intptr_t i = 0; i < len; i++) {
     ic_data.GetCheckAt(i, &class_ids, &target);
     if (class_ids[arg_n] != kSmiCid) return false;
   }
   return true;
 }
 
 
 // Returns array classid to load from, array and index value
 
 intptr_t FlowGraphOptimizer::PrepareIndexedOp(InstanceCallInstr* call,
                                               intptr_t class_id,
-                                              Value** array,
-                                              Value** index) {
-  *array = call->ArgumentAt(0)->value();
-  *index = call->ArgumentAt(1)->value();
+                                              Definition** array,
+                                              Definition** index) {

[Vyacheslav Egorov (Google), 2013/02/08 16:44:49]

it seems the index is not assigned in the body any more. 

you can convert it from Definition** to Definition*

   // Insert class check and index smi checks and attach a copy of the
   // original environment because the operation can still deoptimize.
-  AddCheckClass(call, (*array)->Copy());
+  AddReceiverCheck(call);
   InsertBefore(call,
-               new CheckSmiInstr((*index)->Copy(), call->deopt_id()),
+               new CheckSmiInstr(new Value(*index), call->deopt_id()),
                call->env(),
                Definition::kEffect);
+
   // If both index and array are constants, then do a compile-time check.
   // TODO(srdjan): Remove once constant propagation handles bounds checks.
   bool skip_check = false;
-  if ((*array)->BindsToConstant() && (*index)->BindsToConstant()) {
-    ConstantInstr* array_def = (*array)->definition()->AsConstant();
+  if ((*array)->IsConstant() && (*index)->IsConstant()) {
     const ImmutableArray& constant_array =
-        ImmutableArray::Cast(array_def->value());
-    ConstantInstr* index_def = (*index)->definition()->AsConstant();
-    if (index_def->value().IsSmi()) {
-      intptr_t constant_index = Smi::Cast(index_def->value()).Value();
-      skip_check = (constant_index < constant_array.Length());
-    }
+        ImmutableArray::Cast((*array)->AsConstant()->value());
+    const Object& constant_index = (*index)->AsConstant()->value();
+      skip_check = constant_index.IsSmi() &&
+          (Smi::Cast(constant_index).Value() < constant_array.Length());
   }
   if (!skip_check) {
     // Insert array length load and bounds check.
     const bool is_immutable =
         CheckArrayBoundInstr::IsFixedLengthArrayType(class_id);
-    LoadFieldInstr* length = new LoadFieldInstr(
-        (*array)->Copy(),
-        CheckArrayBoundInstr::LengthOffsetFor(class_id),
-        Type::ZoneHandle(Type::SmiType()),
-        is_immutable);
+    LoadFieldInstr* length =
+        new LoadFieldInstr(new Value(*array),
+                           CheckArrayBoundInstr::LengthOffsetFor(class_id),
+                           Type::ZoneHandle(Type::SmiType()),
+                           is_immutable);
     length->set_result_cid(kSmiCid);
     length->set_recognized_kind(
         LoadFieldInstr::RecognizedKindFromArrayCid(class_id));
     InsertBefore(call, length, NULL, Definition::kValue);
+
     InsertBefore(call,
                  new CheckArrayBoundInstr(new Value(length),
-                                          (*index)->Copy(),
+                                          new Value(*index),
                                           class_id,
                                           call),
                  call->env(),
                  Definition::kEffect);
   }
   if (class_id == kGrowableObjectArrayCid) {
     // Insert data elements load.
     LoadFieldInstr* elements =
-        new LoadFieldInstr((*array)->Copy(),
+        new LoadFieldInstr(new Value(*array),
                            GrowableObjectArray::data_offset(),
                            Type::ZoneHandle(Type::DynamicType()));
     elements->set_result_cid(kArrayCid);
     InsertBefore(call, elements, NULL, Definition::kValue);
-    *array = new Value(elements);
+    *array = elements;
     return kArrayCid;
   }
   return class_id;
 }
 
 
 bool FlowGraphOptimizer::TryReplaceWithStoreIndexed(InstanceCallInstr* call) {
   const intptr_t class_id = ReceiverClassId(call);
   ICData& value_check = ICData::ZoneHandle();
   switch (class_id) {
@@ skipping 45 matching lines @@
           (value_check.GetReceiverClassIdAt(0) != kDoubleCid)) {
         return false;
       }
       break;
     }
     default:
       // TODO(fschneider): Add support for other array types.
       return false;
   }
 
+  Definition* array = call->ArgumentAt(0);
+  Definition* index = call->ArgumentAt(1);
+  Definition* stored_value = call->ArgumentAt(2);
   if (FLAG_enable_type_checks) {
-    Value* array = call->ArgumentAt(0)->value();
-    Value* value = call->ArgumentAt(2)->value();
     // Only type check for the value. A type check for the index is not
     // needed here because we insert a deoptimizing smi-check for the case
     // the index is not a smi.
     const Function& target =
         Function::ZoneHandle(call->ic_data()->GetTargetAt(0));
     const AbstractType& value_type =
         AbstractType::ZoneHandle(target.ParameterTypeAt(2));
-    Value* instantiator = NULL;
-    Value* type_args = NULL;
+    Definition* instantiator = NULL;
+    Definition* type_args = NULL;
     switch (class_id) {
       case kArrayCid:
       case kGrowableObjectArrayCid: {
         const Class& instantiator_class = Class::Handle(target.Owner());
         intptr_t type_arguments_field_offset =
             instantiator_class.type_arguments_field_offset();
         LoadFieldInstr* load_type_args =
-            new LoadFieldInstr(array->Copy(),
+            new LoadFieldInstr(new Value(array),
                                type_arguments_field_offset,
                                Type::ZoneHandle());  // No type.
         InsertBefore(call, load_type_args, NULL, Definition::kValue);
-        instantiator = array->Copy();
-        type_args = new Value(load_type_args);
+        instantiator = array;
+        type_args = load_type_args;
         break;
       }
       case kInt8ArrayCid:
       case kUint8ArrayCid:
       case kUint8ClampedArrayCid:
       case kInt16ArrayCid:
       case kUint16ArrayCid:
       case kInt32ArrayCid:
       case kUint32ArrayCid:
         ASSERT(value_type.IsIntType());
         // Fall through.
       case kFloat32ArrayCid:
       case kFloat64ArrayCid: {
-        instantiator = new Value(flow_graph_->constant_null());
-        type_args = new Value(flow_graph_->constant_null());
+        type_args = instantiator = flow_graph_->constant_null();
         ASSERT((class_id != kFloat32ArrayCid && class_id != kFloat64ArrayCid) ||
                value_type.IsDoubleType());
         ASSERT(value_type.IsInstantiated());
         break;
       }
       default:
         // TODO(fschneider): Add support for other array types.
         UNREACHABLE();
     }
     AssertAssignableInstr* assert_value =
         new AssertAssignableInstr(call->token_pos(),
-                                  value->Copy(),
-                                  instantiator,
-                                  type_args,
+                                  new Value(stored_value),
+                                  new Value(instantiator),
+                                  new Value(type_args),
                                   value_type,
                                   Symbols::Value());
     // Newly inserted instructions that can deoptimize or throw an exception
     // must have a deoptimization id that is valid for lookup in the unoptimized
     // code.
     assert_value->deopt_id_ = call->deopt_id();
     InsertBefore(call, assert_value, call->env(), Definition::kValue);
   }
 
-  Value* array = NULL;
-  Value* index = NULL;
   intptr_t array_cid = PrepareIndexedOp(call, class_id, &array, &index);
-  Value* value = call->ArgumentAt(2)->value();
   // Check if store barrier is needed.
   bool needs_store_barrier = true;
   if (!value_check.IsNull()) {
     needs_store_barrier = false;
-    if (value_check.NumberOfChecks() == 1 &&
-        value_check.GetReceiverClassIdAt(0) == kSmiCid) {
-      InsertBefore(call,
-                   new CheckSmiInstr(value->Copy(), call->deopt_id()),
-                   call->env(),
-                   Definition::kEffect);
-    } else {
-      InsertBefore(call,
-                   new CheckClassInstr(value->Copy(),
-                                       call->deopt_id(),
-                                       value_check),
-                   call->env(),
-                   Definition::kEffect);
-    }
+    AddCheckClass(stored_value, value_check, call->deopt_id(), call->env(),
+                  call);
   }
 
-  Definition* array_op =
-      new StoreIndexedInstr(array, index, value,
-                            needs_store_barrier, array_cid, call->deopt_id());
-  call->ReplaceWith(array_op, current_iterator());
-  RemovePushArguments(call);
+  Definition* array_op = new StoreIndexedInstr(new Value(array),
+                                               new Value(index),
+                                               new Value(stored_value),
+                                               needs_store_barrier,
+                                               array_cid,
+                                               call->deopt_id());
+  ReplaceCall(call, array_op);
   return true;
 }
 
 
 
 bool FlowGraphOptimizer::TryReplaceWithLoadIndexed(InstanceCallInstr* call) {
   const intptr_t class_id = ReceiverClassId(call);
   // Set deopt_id to a valid id if the LoadIndexedInstr can cause deopt.
   intptr_t deopt_id = Isolate::kNoDeoptId;
   switch (class_id) {
@@ skipping 22 matching lines @@
         // Assume mixed Mint/Smi if this instruction caused deoptimization once.
         ASSERT(call->HasICData());
         const ICData& ic_data = *call->ic_data();
         deopt_id = (ic_data.deopt_reason() == kDeoptUnknown) ?
             call->deopt_id() : Isolate::kNoDeoptId;
       }
       break;
     default:
       return false;
   }
-  Value* array = NULL;
-  Value* index = NULL;
+  Definition* array = call->ArgumentAt(0);
+  Definition* index = call->ArgumentAt(1);
   intptr_t array_cid = PrepareIndexedOp(call, class_id, &array, &index);
   Definition* array_op =
-      new LoadIndexedInstr(array,
-                           index,
+      new LoadIndexedInstr(new Value(array),
+                           new Value(index),
                            FlowGraphCompiler::ElementSizeFor(array_cid),
                            array_cid,
                            deopt_id);
-  call->ReplaceWith(array_op, current_iterator());
-  RemovePushArguments(call);
+  ReplaceCall(call, array_op);
   return true;
 }
 
 
-void FlowGraphOptimizer::InsertBefore(Instruction* next,
-                                      Instruction* instr,
-                                      Environment* env,
-                                      Definition::UseKind use_kind) {
-  if (env != NULL) env->DeepCopyTo(instr);
-  if (use_kind == Definition::kValue) {
-    ASSERT(instr->IsDefinition());
-    instr->AsDefinition()->set_ssa_temp_index(
-        flow_graph_->alloc_ssa_temp_index());
-  }
-  instr->InsertBefore(next);
-}
-
-
-void FlowGraphOptimizer::InsertAfter(Instruction* prev,
-                                     Instruction* instr,
-                                     Environment* env,
-                                     Definition::UseKind use_kind) {
-  if (env != NULL) env->DeepCopyTo(instr);
-  if (use_kind == Definition::kValue) {
-    ASSERT(instr->IsDefinition());
-    instr->AsDefinition()->set_ssa_temp_index(
-        flow_graph_->alloc_ssa_temp_index());
-  }
-  instr->InsertAfter(prev);
-}
-
-
 bool FlowGraphOptimizer::TryReplaceWithBinaryOp(InstanceCallInstr* call,
                                                 Token::Kind op_kind) {
   intptr_t operands_type = kIllegalCid;
   ASSERT(call->HasICData());
   const ICData& ic_data = *call->ic_data();
   switch (op_kind) {
     case Token::kADD:
     case Token::kSUB:
       if (HasOnlyTwoSmis(ic_data)) {
         // Don't generate smi code if the IC data is marked because
@@ skipping 76 matching lines @@
     case Token::kTRUNCDIV:
       if (HasOnlyTwoSmis(ic_data)) {
         if (ic_data.deopt_reason() == kDeoptBinarySmiOp) return false;
         operands_type = kSmiCid;
       } else {
         return false;
       }
       break;
     default:
       UNREACHABLE();
-  };
+  }
 
   ASSERT(call->ArgumentCount() == 2);
+  Definition* left = call->ArgumentAt(0);
+  Definition* right = call->ArgumentAt(1);
   if (operands_type == kDoubleCid) {
-    Value* left = call->ArgumentAt(0)->value();
-    Value* right = call->ArgumentAt(1)->value();
-
     // Check that either left or right are not a smi.  Result or a
     // binary operation with two smis is a smi not a double.
     InsertBefore(call,
-                 new CheckEitherNonSmiInstr(left->Copy(),
-                                            right->Copy(),
+                 new CheckEitherNonSmiInstr(new Value(left),
+                                            new Value(right),
                                             call),
                  call->env(),
                  Definition::kEffect);
 
     BinaryDoubleOpInstr* double_bin_op =
-        new BinaryDoubleOpInstr(op_kind, left->Copy(), right->Copy(), call);
-    call->ReplaceWith(double_bin_op, current_iterator());
-    RemovePushArguments(call);
+        new BinaryDoubleOpInstr(op_kind, new Value(left), new Value(right),
+                                call);
+    ReplaceCall(call, double_bin_op);
   } else if (operands_type == kMintCid) {
     if (!FlowGraphCompiler::SupportsUnboxedMints()) return false;
-    Value* left = call->ArgumentAt(0)->value();
-    Value* right = call->ArgumentAt(1)->value();
     if ((op_kind == Token::kSHR) || (op_kind == Token::kSHL)) {
       ShiftMintOpInstr* shift_op =
-          new ShiftMintOpInstr(op_kind, left, right, call);
-      call->ReplaceWith(shift_op, current_iterator());
+          new ShiftMintOpInstr(op_kind, new Value(left), new Value(right),
+                               call);
+      ReplaceCall(call, shift_op);
     } else {
       BinaryMintOpInstr* bin_op =
-          new BinaryMintOpInstr(op_kind, left, right, call);
-      call->ReplaceWith(bin_op, current_iterator());
+          new BinaryMintOpInstr(op_kind, new Value(left), new Value(right),
+                                call);
+      ReplaceCall(call, bin_op);
     }
-    RemovePushArguments(call);
   } else if (op_kind == Token::kMOD) {
     // TODO(vegorov): implement fast path code for modulo.
     ASSERT(operands_type == kSmiCid);
-    if (!call->ArgumentAt(1)->value()->BindsToConstant()) return false;
-    const Object& obj = call->ArgumentAt(1)->value()->BoundConstant();
+    if (!right->IsConstant()) return false;
+    const Object& obj = right->AsConstant()->value();
     if (!obj.IsSmi()) return false;
     const intptr_t value = Smi::Cast(obj).Value();
-    if ((value > 0) && Utils::IsPowerOfTwo(value)) {
-      Value* left = call->ArgumentAt(0)->value();
-      // Insert smi check and attach a copy of the original
-      // environment because the smi operation can still deoptimize.
-      InsertBefore(call,
-                   new CheckSmiInstr(left->Copy(), call->deopt_id()),
-                   call->env(),
-                   Definition::kEffect);
-      ConstantInstr* c = new ConstantInstr(Smi::Handle(Smi::New(value - 1)));
-      InsertBefore(call, c, NULL, Definition::kValue);
-      BinarySmiOpInstr* bin_op =
-          new BinarySmiOpInstr(Token::kBIT_AND, call, left, new Value(c));
-      call->ReplaceWith(bin_op, current_iterator());
-      RemovePushArguments(call);
-    } else {
-      // Did not replace.
-      return false;
-    }
+    if ((value <= 0) || !Utils::IsPowerOfTwo(value)) return false;
+
+    // Insert smi check and attach a copy of the original environment
+    // because the smi operation can still deoptimize.
+    InsertBefore(call,
+                 new CheckSmiInstr(new Value(left), call->deopt_id()),
+                 call->env(),
+                 Definition::kEffect);
+    ConstantInstr* constant =
+        new ConstantInstr(Smi::Handle(Smi::New(value - 1)));
+    InsertBefore(call, constant, NULL, Definition::kValue);
+    BinarySmiOpInstr* bin_op =
+        new BinarySmiOpInstr(Token::kBIT_AND, call,
+                             new Value(left),
+                             new Value(constant));
+    ReplaceCall(call, bin_op);
   } else {
     ASSERT(operands_type == kSmiCid);
-    Value* left = call->ArgumentAt(0)->value();
-    Value* right = call->ArgumentAt(1)->value();
     // Insert two smi checks and attach a copy of the original
     // environment because the smi operation can still deoptimize.
     InsertBefore(call,
-                 new CheckSmiInstr(left->Copy(), call->deopt_id()),
+                 new CheckSmiInstr(new Value(left), call->deopt_id()),
                  call->env(),
                  Definition::kEffect);
     InsertBefore(call,
-                 new CheckSmiInstr(right->Copy(), call->deopt_id()),
+                 new CheckSmiInstr(new Value(right), call->deopt_id()),
                  call->env(),
                  Definition::kEffect);
-    if (left->BindsToConstant() &&
+    if (left->IsConstant() &&
         ((op_kind == Token::kADD) || (op_kind == Token::kMUL))) {
       // Constant should be on the right side.
-      Value* temp = left;
+      Definition* temp = left;
       left = right;
       right = temp;
     }
-    BinarySmiOpInstr* bin_op = new BinarySmiOpInstr(op_kind, call, left, right);
-    call->ReplaceWith(bin_op, current_iterator());
-    RemovePushArguments(call);
+    BinarySmiOpInstr* bin_op =
+        new BinarySmiOpInstr(op_kind, call, new Value(left), new Value(right));
+    ReplaceCall(call, bin_op);
   }
   return true;
 }
 
 
 bool FlowGraphOptimizer::TryReplaceWithUnaryOp(InstanceCallInstr* call,
                                                Token::Kind op_kind) {
   ASSERT(call->ArgumentCount() == 1);
+  Definition* input = call->ArgumentAt(0);
   Definition* unary_op = NULL;
   if (HasOnlyOneSmi(*call->ic_data())) {
-    Value* value = call->ArgumentAt(0)->value();
     InsertBefore(call,
-                 new CheckSmiInstr(value->Copy(), call->deopt_id()),
+                 new CheckSmiInstr(new Value(input), call->deopt_id()),
                  call->env(),
                  Definition::kEffect);
-    unary_op = new UnarySmiOpInstr(op_kind, call, value);
+    unary_op = new UnarySmiOpInstr(op_kind, call, new Value(input));
   } else if ((op_kind == Token::kBIT_NOT) &&
              HasOnlySmiOrMint(*call->ic_data()) &&
              FlowGraphCompiler::SupportsUnboxedMints()) {
-    Value* value = call->ArgumentAt(0)->value();
-    unary_op = new UnaryMintOpInstr(op_kind, value, call);
+    unary_op = new UnaryMintOpInstr(op_kind, new Value(input), call);
   } else if (HasOnlyOneDouble(*call->ic_data()) &&
              (op_kind == Token::kNEGATE)) {
-    Value* value = call->ArgumentAt(0)->value();
-    AddCheckClass(call, value->Copy());
+    AddReceiverCheck(call);
     ConstantInstr* minus_one =
         new ConstantInstr(Double::ZoneHandle(Double::NewCanonical(-1)));
     InsertBefore(call, minus_one, NULL, Definition::kValue);
     unary_op = new BinaryDoubleOpInstr(Token::kMUL,
-                                       value,
+                                       new Value(input),
                                        new Value(minus_one),
                                        call);
   }
   if (unary_op == NULL) return false;
 
-  call->ReplaceWith(unary_op, current_iterator());
-  RemovePushArguments(call);
+  ReplaceCall(call, unary_op);
   return true;
 }
 
 
 // Using field class
 static RawField* GetField(intptr_t class_id, const String& field_name) {
   Class& cls = Class::Handle(Isolate::Current()->class_table()->At(class_id));
   Field& field = Field::Handle();
   while (!cls.IsNull()) {
     field = cls.LookupInstanceField(field_name);
     if (!field.IsNull()) {
       return field.raw();
     }
     cls = cls.SuperClass();
   }
   return Field::null();
 }
 
 
 // Use CHA to determine if the call needs a class check: if the callee's
 // receiver is the same as the caller's receiver and there are no overriden
 // callee functions, then no class check is needed.
 bool FlowGraphOptimizer::InstanceCallNeedsClassCheck(
     InstanceCallInstr* call) const {
   if (!FLAG_use_cha) return true;
-  Definition* callee_receiver = call->ArgumentAt(0)->value()->definition();
+  Definition* callee_receiver = call->ArgumentAt(0);
   ASSERT(callee_receiver != NULL);
   const Function& function = flow_graph_->parsed_function().function();
   if (function.IsDynamicFunction() &&
       callee_receiver->IsParameter() &&
       (callee_receiver->AsParameter()->index() == 0)) {
     return CHA::HasOverride(Class::Handle(function.Owner()),
                             call->function_name());
   }
   return true;
 }
 
 
 bool FlowGraphOptimizer::MethodExtractorNeedsClassCheck(
     InstanceCallInstr* call) const {
   if (!FLAG_use_cha) return true;
-  Definition* callee_receiver = call->ArgumentAt(0)->value()->definition();
+  Definition* callee_receiver = call->ArgumentAt(0);
   ASSERT(callee_receiver != NULL);
   const Function& function = flow_graph_->parsed_function().function();
   if (function.IsDynamicFunction() &&
       callee_receiver->IsParameter() &&
       (callee_receiver->AsParameter()->index() == 0)) {
     const String& field_name =
       String::Handle(Field::NameFromGetter(call->function_name()));
     return CHA::HasOverride(Class::Handle(function.Owner()), field_name);
   }
   return true;
 }
 
 
 void FlowGraphOptimizer::InlineImplicitInstanceGetter(InstanceCallInstr* call) {
   ASSERT(call->HasICData());
   const ICData& ic_data = *call->ic_data();
   Function& target = Function::Handle();
   GrowableArray<intptr_t> class_ids;
   ic_data.GetCheckAt(0, &class_ids, &target);
   ASSERT(class_ids.length() == 1);
   // Inline implicit instance getter.
   const String& field_name =
       String::Handle(Field::NameFromGetter(call->function_name()));
   const Field& field = Field::Handle(GetField(class_ids[0], field_name));
   ASSERT(!field.IsNull());
 
   if (InstanceCallNeedsClassCheck(call)) {
-    AddCheckClass(call, call->ArgumentAt(0)->value()->Copy());
+    AddReceiverCheck(call);
   }
-  // Detach environment from the original instruction because it can't
-  // deoptimize.
-  call->set_env(NULL);
   LoadFieldInstr* load = new LoadFieldInstr(
-      call->ArgumentAt(0)->value(),
+      new Value(call->ArgumentAt(0)),
       field.Offset(),
       AbstractType::ZoneHandle(field.type()),
       field.is_final());
-  call->ReplaceWith(load, current_iterator());
-  RemovePushArguments(call);
+  // Detach environment from the original instruction because it can't
+  // deoptimize.
+  for (Environment::DeepIterator it(call->env()); !it.Done(); it.Advance()) {
+    it.CurrentValue()->RemoveFromUseList();
+  }
+  call->set_env(NULL);
+  ReplaceCall(call, load);
 }
 
 
 void FlowGraphOptimizer::InlineArrayLengthGetter(InstanceCallInstr* call,
                                                  intptr_t length_offset,
                                                  bool is_immutable,
                                                  MethodRecognizer::Kind kind) {
-  // Check receiver class.
-  AddCheckClass(call, call->ArgumentAt(0)->value()->Copy());
+  AddReceiverCheck(call);
 
   LoadFieldInstr* load = new LoadFieldInstr(
-      call->ArgumentAt(0)->value(),
+      new Value(call->ArgumentAt(0)),
       length_offset,
       Type::ZoneHandle(Type::SmiType()),
       is_immutable);
   load->set_result_cid(kSmiCid);
   load->set_recognized_kind(kind);
-  call->ReplaceWith(load, current_iterator());
-  RemovePushArguments(call);
+  ReplaceCall(call, load);
 }
 
 
 void FlowGraphOptimizer::InlineGrowableArrayCapacityGetter(
     InstanceCallInstr* call) {
-  // Check receiver class.
-  AddCheckClass(call, call->ArgumentAt(0)->value()->Copy());
+  AddReceiverCheck(call);
 
   // TODO(srdjan): type of load should be GrowableObjectArrayType.
   LoadFieldInstr* data_load = new LoadFieldInstr(
-      call->ArgumentAt(0)->value(),
+      new Value(call->ArgumentAt(0)),
       Array::data_offset(),
       Type::ZoneHandle(Type::DynamicType()));
   data_load->set_result_cid(kArrayCid);
   InsertBefore(call, data_load, NULL, Definition::kValue);
 
   LoadFieldInstr* length_load = new LoadFieldInstr(
       new Value(data_load),
       Array::length_offset(),
       Type::ZoneHandle(Type::SmiType()));
   length_load->set_result_cid(kSmiCid);
   length_load->set_recognized_kind(MethodRecognizer::kObjectArrayLength);
 
-  call->ReplaceWith(length_load, current_iterator());
-  RemovePushArguments(call);
+  ReplaceCall(call, length_load);
 }
 
 
-static LoadFieldInstr* BuildLoadStringLength(Value* str) {
+static LoadFieldInstr* BuildLoadStringLength(Definition* str) {
   const bool is_immutable = true;  // String length is immutable.
   LoadFieldInstr* load = new LoadFieldInstr(
-      str,
+      new Value(str),
       String::length_offset(),
       Type::ZoneHandle(Type::SmiType()),
       is_immutable);
   load->set_result_cid(kSmiCid);
   return load;
 }
 
 
 void FlowGraphOptimizer::InlineStringLengthGetter(InstanceCallInstr* call) {
-  // Check receiver class.
-  AddCheckClass(call, call->ArgumentAt(0)->value()->Copy());
+  AddReceiverCheck(call);
 
-  LoadFieldInstr* load = BuildLoadStringLength(call->ArgumentAt(0)->value());
+  LoadFieldInstr* load = BuildLoadStringLength(call->ArgumentAt(0));
   load->set_recognized_kind(MethodRecognizer::kStringBaseLength);
-  call->ReplaceWith(load, current_iterator());
-  RemovePushArguments(call);
+  ReplaceCall(call, load);
 }
 
 
 void FlowGraphOptimizer::InlineStringIsEmptyGetter(InstanceCallInstr* call) {
-  // Check receiver class.
-  AddCheckClass(call, call->ArgumentAt(0)->value()->Copy());
+  AddReceiverCheck(call);
 
-  LoadFieldInstr* load = BuildLoadStringLength(call->ArgumentAt(0)->value());
+  LoadFieldInstr* load = BuildLoadStringLength(call->ArgumentAt(0));
   InsertBefore(call, load, NULL, Definition::kValue);
 
   ConstantInstr* zero = new ConstantInstr(Smi::Handle(Smi::New(0)));
   InsertBefore(call, zero, NULL, Definition::kValue);
 
   StrictCompareInstr* compare =
       new StrictCompareInstr(Token::kEQ_STRICT,
                              new Value(load),
                              new Value(zero));
-  call->ReplaceWith(compare, current_iterator());
-  RemovePushArguments(call);
+  ReplaceCall(call, compare);
 }
 
 
 static intptr_t OffsetForLengthGetter(MethodRecognizer::Kind kind) {
   switch (kind) {
     case MethodRecognizer::kObjectArrayLength:
     case MethodRecognizer::kImmutableArrayLength:
       return Array::length_offset();
     case MethodRecognizer::kByteArrayBaseLength:
       return ByteArray::length_offset();
@@ skipping 70 matching lines @@
     default:
       ASSERT(recognized_kind == MethodRecognizer::kUnknown);
   }
   return false;
 }
 
 
 LoadIndexedInstr* FlowGraphOptimizer::BuildStringCharCodeAt(
     InstanceCallInstr* call,
     intptr_t cid) {
-  Value* str = call->ArgumentAt(0)->value();
-  Value* index = call->ArgumentAt(1)->value();
-  AddCheckClass(call, str->Copy());
+  Definition* str = call->ArgumentAt(0);
+  Definition* index = call->ArgumentAt(1);
+  AddReceiverCheck(call);
   InsertBefore(call,
-               new CheckSmiInstr(index->Copy(), call->deopt_id()),
+               new CheckSmiInstr(new Value(index), call->deopt_id()),
                call->env(),
                Definition::kEffect);
   // If both index and string are constants, then do a compile-time check.
   // TODO(srdjan): Remove once constant propagation handles bounds checks.
   bool skip_check = false;
-  if (str->BindsToConstant() && index->BindsToConstant()) {
-    ConstantInstr* string_def = str->definition()->AsConstant();
+  if (str->IsConstant() && index->IsConstant()) {
     const String& constant_string =
-        String::Cast(string_def->value());
-    ConstantInstr* index_def = index->definition()->AsConstant();
-    if (index_def->value().IsSmi()) {
-      intptr_t constant_index = Smi::Cast(index_def->value()).Value();
-      skip_check = (constant_index < constant_string.Length());
-    }
+        String::Cast(str->AsConstant()->value());
+    const Object& constant_index = index->AsConstant()->value();
+    skip_check = constant_index.IsSmi() &&
+        (Smi::Cast(constant_index).Value() < constant_string.Length());
   }
   if (!skip_check) {
     // Insert bounds check.
     const bool is_immutable = true;
     LoadFieldInstr* length = new LoadFieldInstr(
-        str->Copy(),
+        new Value(str),
         CheckArrayBoundInstr::LengthOffsetFor(cid),
         Type::ZoneHandle(Type::SmiType()),
         is_immutable);
     length->set_result_cid(kSmiCid);
     length->set_recognized_kind(MethodRecognizer::kStringBaseLength);
     InsertBefore(call, length, NULL, Definition::kValue);
     InsertBefore(call,
                  new CheckArrayBoundInstr(new Value(length),
-                                          index->Copy(),
+                                          new Value(index),
                                           cid,
                                           call),
                  call->env(),
                  Definition::kEffect);
   }
-  return new LoadIndexedInstr(str,
-                              index,
+  return new LoadIndexedInstr(new Value(str),
+                              new Value(index),
                               FlowGraphCompiler::ElementSizeFor(cid),
                               cid,
                               Isolate::kNoDeoptId);  // Can't deoptimize.
 }
 
 
 void FlowGraphOptimizer::ReplaceWithMathCFunction(
   InstanceCallInstr* call,
   MethodRecognizer::Kind recognized_kind) {
-  AddCheckClass(call, call->ArgumentAt(0)->value()->Copy());
+  AddReceiverCheck(call);
   ZoneGrowableArray<Value*>* args =
       new ZoneGrowableArray<Value*>(call->ArgumentCount());
   for (intptr_t i = 0; i < call->ArgumentCount(); i++) {
-    args->Add(call->ArgumentAt(i)->value());
+    args->Add(new Value(call->ArgumentAt(i)));
   }
   InvokeMathCFunctionInstr* invoke =
       new InvokeMathCFunctionInstr(args, call, recognized_kind);
-  call->ReplaceWith(invoke, current_iterator());
-  RemovePushArguments(call);
+  ReplaceCall(call, invoke);
 }
 
 
 static bool IsSupportedByteArrayCid(intptr_t cid) {
   switch (cid) {
     case kInt8ArrayCid:
     case kUint8ArrayCid:
     case kUint8ClampedArrayCid:
     case kInt16ArrayCid:
     case kUint16ArrayCid:
@@ skipping 19 matching lines @@
   Function& target = Function::Handle();
   GrowableArray<intptr_t> class_ids;
   ic_data.GetCheckAt(0, &class_ids, &target);
   MethodRecognizer::Kind recognized_kind =
       MethodRecognizer::RecognizeKind(target);
   if ((recognized_kind == MethodRecognizer::kStringBaseCharCodeAt) &&
       (ic_data.NumberOfChecks() == 1) &&
       ((class_ids[0] == kOneByteStringCid) ||
        (class_ids[0] == kTwoByteStringCid))) {
     LoadIndexedInstr* instr = BuildStringCharCodeAt(call, class_ids[0]);
-    call->ReplaceWith(instr, current_iterator());
-    RemovePushArguments(call);
+    ReplaceCall(call, instr);
     return true;
   }
   if ((recognized_kind == MethodRecognizer::kStringBaseCharAt) &&
       (ic_data.NumberOfChecks() == 1) &&
       (class_ids[0] == kOneByteStringCid)) {
     // TODO(fschneider): Handle TwoByteString.
     LoadIndexedInstr* load_char_code =
         BuildStringCharCodeAt(call, class_ids[0]);
     InsertBefore(call, load_char_code, NULL, Definition::kValue);
     StringFromCharCodeInstr* char_at =
         new StringFromCharCodeInstr(new Value(load_char_code),
                                     kOneByteStringCid);
-    call->ReplaceWith(char_at, current_iterator());
-    RemovePushArguments(call);
+    ReplaceCall(call, char_at);
     return true;
   }
 
   if ((recognized_kind == MethodRecognizer::kIntegerToDouble) &&
       (class_ids[0] == kSmiCid)) {
     SmiToDoubleInstr* s2d_instr = new SmiToDoubleInstr(call);
     call->ReplaceWith(s2d_instr, current_iterator());
     // Pushed arguments are not removed because SmiToDouble is implemented
     // as a call.
     return true;
   }
 
   if (class_ids[0] == kDoubleCid) {
     switch (recognized_kind) {
       case MethodRecognizer::kDoubleToInteger: {
-        AddCheckClass(call, call->ArgumentAt(0)->value()->Copy());
+        AddReceiverCheck(call);
         ASSERT(call->HasICData());
         const ICData& ic_data = *call->ic_data();
+        Definition* input = call->ArgumentAt(0);
         Definition* d2i_instr = NULL;
         if (ic_data.deopt_reason() == kDeoptDoubleToSmi) {
           // Do not repeatedly deoptimize because result didn't fit into Smi.
-          d2i_instr = new DoubleToIntegerInstr(call->ArgumentAt(0)->value(),
-                                               call);
+          d2i_instr = new DoubleToIntegerInstr(new Value(input), call);
         } else {
           // Optimistically assume result fits into Smi.
-          d2i_instr = new DoubleToSmiInstr(call->ArgumentAt(0)->value(), call);
+          d2i_instr = new DoubleToSmiInstr(new Value(input), call);
         }
-        call->ReplaceWith(d2i_instr, current_iterator());
-        RemovePushArguments(call);
+        ReplaceCall(call, d2i_instr);
         return true;
       }
       case MethodRecognizer::kDoubleMod:
       case MethodRecognizer::kDoublePow:
         ReplaceWithMathCFunction(call, recognized_kind);
         return true;
       case MethodRecognizer::kDoubleTruncate:
       case MethodRecognizer::kDoubleRound:
       case MethodRecognizer::kDoubleFloor:
       case MethodRecognizer::kDoubleCeil:
         if (!CPUFeatures::double_truncate_round_supported()) {
           ReplaceWithMathCFunction(call, recognized_kind);
         } else {
-          AddCheckClass(call, call->ArgumentAt(0)->value()->Copy());
+          AddReceiverCheck(call);
           DoubleToDoubleInstr* d2d_instr =
-              new DoubleToDoubleInstr(call->ArgumentAt(0)->value(),
+              new DoubleToDoubleInstr(new Value(call->ArgumentAt(0)),
                                       call,
                                       recognized_kind);
-          call->ReplaceWith(d2d_instr, current_iterator());
-          RemovePushArguments(call);
+          ReplaceCall(call, d2d_instr);
         }
         return true;
       default:
         // Unsupported method.
         return false;
     }
   }
 
   if (IsSupportedByteArrayCid(class_ids[0]) &&
       (ic_data.NumberOfChecks() == 1)) {
@@ skipping 21 matching lines @@
         array_op = BuildByteArrayViewLoad(call, class_ids[0], kFloat32ArrayCid);
         break;
       case MethodRecognizer::kByteArrayBaseGetFloat64:
         array_op = BuildByteArrayViewLoad(call, class_ids[0], kFloat64ArrayCid);
         break;
       default:
         // Unsupported method.
         return false;
     }
     ASSERT(array_op != NULL);
-    call->ReplaceWith(array_op, current_iterator());
-    RemovePushArguments(call);
+    ReplaceCall(call, array_op);
     return true;
   }
   return false;
 }
 
 
 LoadIndexedInstr* FlowGraphOptimizer::BuildByteArrayViewLoad(
     InstanceCallInstr* call,
     intptr_t receiver_cid,
     intptr_t view_cid) {
-    Value* array = call->ArgumentAt(0)->value();
-    Value* byte_index = call->ArgumentAt(1)->value();
+  Definition* array = call->ArgumentAt(0);
+  Definition* byte_index = call->ArgumentAt(1);
 
-    AddCheckClass(call, array->Copy());
-    const bool is_immutable = true;
-    LoadFieldInstr* length = new LoadFieldInstr(
-        array->Copy(),
-        CheckArrayBoundInstr::LengthOffsetFor(receiver_cid),
-        Type::ZoneHandle(Type::SmiType()),
-        is_immutable);
-    length->set_result_cid(kSmiCid);
-    length->set_recognized_kind(
-        LoadFieldInstr::RecognizedKindFromArrayCid(receiver_cid));
-    InsertBefore(call, length, NULL, Definition::kValue);
+  AddReceiverCheck(call);
+  const bool is_immutable = true;
+  LoadFieldInstr* length = new LoadFieldInstr(
+      new Value(array),
+      CheckArrayBoundInstr::LengthOffsetFor(receiver_cid),
+      Type::ZoneHandle(Type::SmiType()),
+      is_immutable);
+  length->set_result_cid(kSmiCid);
+  length->set_recognized_kind(
+      LoadFieldInstr::RecognizedKindFromArrayCid(receiver_cid));
+  InsertBefore(call, length, NULL, Definition::kValue);
 
-    // len_in_bytes = length * kBytesPerElement(receiver)
-    intptr_t element_size = FlowGraphCompiler::ElementSizeFor(receiver_cid);
-    ConstantInstr* bytes_per_element =
-        new ConstantInstr(Smi::Handle(Smi::New(element_size)));
-    InsertBefore(call, bytes_per_element, NULL, Definition::kValue);
-    BinarySmiOpInstr* len_in_bytes =
-        new BinarySmiOpInstr(Token::kMUL,
-                             call,
-                             new Value(length),
-                             new Value(bytes_per_element));
-    InsertBefore(call, len_in_bytes, call->env(), Definition::kValue);
+  // len_in_bytes = length * kBytesPerElement(receiver)
+  intptr_t element_size = FlowGraphCompiler::ElementSizeFor(receiver_cid);
+  ConstantInstr* bytes_per_element =
+      new ConstantInstr(Smi::Handle(Smi::New(element_size)));
+  InsertBefore(call, bytes_per_element, NULL, Definition::kValue);
+  BinarySmiOpInstr* len_in_bytes =
+      new BinarySmiOpInstr(Token::kMUL,
+                           call,
+                           new Value(length),
+                           new Value(bytes_per_element));
+  InsertBefore(call, len_in_bytes, call->env(), Definition::kValue);
 
     // Check byte_index < len_in_bytes.
-    InsertBefore(call,
-                 new CheckArrayBoundInstr(new Value(len_in_bytes),
-                                          byte_index->Copy(),
-                                          receiver_cid,
-                                          call),
-                 call->env(),
-                 Definition::kEffect);
+  InsertBefore(call,
+               new CheckArrayBoundInstr(new Value(len_in_bytes),
+                                        new Value(byte_index),
+                                        receiver_cid,
+                                        call),
+               call->env(),
+               Definition::kEffect);
 
-    // TODO(fschneider): Optimistically build smi load for Int32 and Uint32
-    // loads on ia32 like we do for normal array loads, and only revert to
-    // mint case after deoptimizing here.
-    return new LoadIndexedInstr(array,
-                                byte_index,
-                                1,  // Index scale.
-                                view_cid,
-                                Isolate::kNoDeoptId);  // Can't deoptimize.
+  // TODO(fschneider): Optimistically build smi load for Int32 and Uint32
+  // loads on ia32 like we do for normal array loads, and only revert to
+  // mint case after deoptimizing here.
+  return new LoadIndexedInstr(new Value(array),
+                              new Value(byte_index),
+                              1,  // Index scale.
+                              view_cid,
+                              Isolate::kNoDeoptId);  // Can't deoptimize.
 }
 
 
 // Returns a Boolean constant if all classes in ic_data yield the same type-test
 // result and the type tests do not depend on type arguments. Otherwise return
 // Bool::null().
 RawBool* FlowGraphOptimizer::InstanceOfAsBool(const ICData& ic_data,
                                               const AbstractType& type) const {
   ASSERT(ic_data.num_args_tested() == 1);  // Unary checks only.
   if (!type.IsInstantiated() || type.IsMalformed()) return Bool::null();
@@ skipping 15 matching lines @@
       if (is_subtype != prev.value()) return Bool::null();
     }
   }
   return prev.raw();
 }
 
 
 // TODO(srdjan): Use ICData to check if always true or false.
 void FlowGraphOptimizer::ReplaceWithInstanceOf(InstanceCallInstr* call) {
   ASSERT(Token::IsTypeTestOperator(call->token_kind()));
-  Value* left_val = call->ArgumentAt(0)->value();
-  Value* instantiator_val = call->ArgumentAt(1)->value();
-  Value* type_args_val = call->ArgumentAt(2)->value();
+  Definition* left = call->ArgumentAt(0);
+  Definition* instantiator = call->ArgumentAt(1);
+  Definition* type_args = call->ArgumentAt(2);
   const AbstractType& type =
-      AbstractType::Cast(call->ArgumentAt(3)->value()->BoundConstant());
+      AbstractType::Cast(call->ArgumentAt(3)->AsConstant()->value());
   const bool negate =
-      Bool::Cast(call->ArgumentAt(4)->value()->BoundConstant()).value();
+      Bool::Cast(call->ArgumentAt(4)->AsConstant()->value()).value();
   const ICData& unary_checks =
       ICData::ZoneHandle(call->ic_data()->AsUnaryClassChecks());
   if (unary_checks.NumberOfChecks() <= FLAG_max_polymorphic_checks) {
     Bool& as_bool = Bool::ZoneHandle(InstanceOfAsBool(unary_checks, type));
     if (!as_bool.IsNull()) {
-      AddCheckClass(call, left_val->Copy());
+      AddReceiverCheck(call);
       if (negate) {
-        as_bool = as_bool.value() ? Bool::False().raw() : Bool::True().raw();
+        as_bool = Bool::Get(!as_bool.value());
       }
       ConstantInstr* bool_const = new ConstantInstr(as_bool);
-      call->ReplaceWith(bool_const, current_iterator());
-      RemovePushArguments(call);
+      ReplaceCall(call, bool_const);
       return;
     }
   }
   InstanceOfInstr* instance_of =
       new InstanceOfInstr(call->token_pos(),
-                          left_val,
-                          instantiator_val,
-                          type_args_val,
+                          new Value(left),
+                          new Value(instantiator),
+                          new Value(type_args),
                           type,
                           negate);
-  call->ReplaceWith(instance_of, current_iterator());
-  RemovePushArguments(call);
+  ReplaceCall(call, instance_of);
 }
 
 
 // Tries to optimize instance call by replacing it with a faster instruction
 // (e.g, binary op, field load, ..).
 void FlowGraphOptimizer::VisitInstanceCall(InstanceCallInstr* instr) {
   if (!instr->HasICData() || (instr->ic_data()->NumberOfChecks() == 0)) {
-    // An instance call without ICData will trigger deoptimization.
     return;
   }
 
   const Token::Kind op_kind = instr->token_kind();
   // Type test is special as it always gets converted into inlined code.
   if (Token::IsTypeTestOperator(op_kind)) {
     ReplaceWithInstanceOf(instr);
     return;
   }
 
@@ skipping 47 matching lines @@
                                            call_with_checks);
       instr->ReplaceWith(call, current_iterator());
       return;
     }
   }
 
   if (unary_checks.NumberOfChecks() <= FLAG_max_polymorphic_checks) {
     bool call_with_checks;
     if (has_one_target) {
       // Type propagation has not run yet, we cannot eliminate the check.
-      AddCheckClass(instr, instr->ArgumentAt(0)->value()->Copy());
+      AddReceiverCheck(instr);
       // Call can still deoptimize, do not detach environment from instr.
       call_with_checks = false;
     } else {
       call_with_checks = true;
     }
     PolymorphicInstanceCallInstr* call =
         new PolymorphicInstanceCallInstr(instr, unary_checks,
                                          call_with_checks);
     instr->ReplaceWith(call, current_iterator());
   }
 }
 
 
 void FlowGraphOptimizer::VisitStaticCall(StaticCallInstr* call) {
   MethodRecognizer::Kind recognized_kind =
       MethodRecognizer::RecognizeKind(call->function());
   if (recognized_kind == MethodRecognizer::kMathSqrt) {
-    MathSqrtInstr* sqrt = new MathSqrtInstr(call->ArgumentAt(0)->value(), call);
-    call->ReplaceWith(sqrt, current_iterator());
-    RemovePushArguments(call);
+    MathSqrtInstr* sqrt =
+        new MathSqrtInstr(new Value(call->ArgumentAt(0)), call);
+    ReplaceCall(call, sqrt);
   }
 }
 
 
 bool FlowGraphOptimizer::TryInlineInstanceSetter(InstanceCallInstr* instr,
                                                  const ICData& unary_ic_data) {
   ASSERT((unary_ic_data.NumberOfChecks() > 0) &&
       (unary_ic_data.num_args_tested() == 1));
   if (FLAG_enable_type_checks) {
     // TODO(srdjan): Add assignable check node if --enable_type_checks.
@@ skipping 17 matching lines @@
     // TODO(srdjan): Inline special setters.
     return false;
   }
   // Inline implicit instance setter.
   const String& field_name =
       String::Handle(Field::NameFromSetter(instr->function_name()));
   const Field& field = Field::Handle(GetField(class_id, field_name));
   ASSERT(!field.IsNull());
 
   if (InstanceCallNeedsClassCheck(instr)) {
-    AddCheckClass(instr, instr->ArgumentAt(0)->value()->Copy());
+    AddReceiverCheck(instr);
   }
   bool needs_store_barrier = true;
   if (ArgIsAlwaysSmi(*instr->ic_data(), 1)) {
     InsertBefore(instr,
-                 new CheckSmiInstr(instr->ArgumentAt(1)->value()->Copy(),
+                 new CheckSmiInstr(new Value(instr->ArgumentAt(1)),
                                    instr->deopt_id()),
                  instr->env(),
                  Definition::kEffect);
     needs_store_barrier = false;
   }
+  StoreInstanceFieldInstr* store = new StoreInstanceFieldInstr(
+      field,
+      new Value(instr->ArgumentAt(0)),
+      new Value(instr->ArgumentAt(1)),
+      needs_store_barrier);
   // Detach environment from the original instruction because it can't
   // deoptimize.
+  for (Environment::DeepIterator it(instr->env()); !it.Done(); it.Advance()) {
+    it.CurrentValue()->RemoveFromUseList();
+  }
   instr->set_env(NULL);

[Vyacheslav Egorov (Google), 2013/02/08 16:44:49]

Loop + set_env(NULL) can be factored out into a helper (used in two places).

-  StoreInstanceFieldInstr* store = new StoreInstanceFieldInstr(
-      field,
-      instr->ArgumentAt(0)->value(),
-      instr->ArgumentAt(1)->value(),
-      needs_store_barrier);
-  instr->ReplaceWith(store, current_iterator());
-  RemovePushArguments(instr);
+  ReplaceCall(instr, store);
   return true;
 }
 
 
-static void HandleRelationalOp(FlowGraphOptimizer* optimizer,
-                               RelationalOpInstr* comp,
-                               Instruction* instr) {
+void FlowGraphOptimizer::HandleRelationalOp(RelationalOpInstr* comp) {
   if (!comp->HasICData() || (comp->ic_data()->NumberOfChecks() == 0)) {
     return;
   }
   const ICData& ic_data = *comp->ic_data();
+  Instruction* instr = current_iterator()->Current();

[Vyacheslav Egorov (Google), 2013/02/08 16:44:49]

This is somewhat confusing. 

Consider calling HandleRelationalOp as HandleCurrentRelationalOp or something
else to indicate that it depends on the current position of the optimizer.

Or better yet, still pass branch into it.

   if (ic_data.NumberOfChecks() == 1) {
     ASSERT(ic_data.HasOneTarget());
     if (HasOnlyTwoSmis(ic_data)) {
-      optimizer->InsertBefore(
-          instr,
-          new CheckSmiInstr(comp->left()->Copy(), comp->deopt_id()),
-          instr->env(),
-          Definition::kEffect);
-      optimizer->InsertBefore(
-          instr,
-          new CheckSmiInstr(comp->right()->Copy(), comp->deopt_id()),
-          instr->env(),
-          Definition::kEffect);
+      InsertBefore(instr,
+                   new CheckSmiInstr(comp->left()->Copy(), comp->deopt_id()),
+                   instr->env(),
+                   Definition::kEffect);
+      InsertBefore(instr,
+                   new CheckSmiInstr(comp->right()->Copy(), comp->deopt_id()),
+                   instr->env(),
+                   Definition::kEffect);
       comp->set_operands_class_id(kSmiCid);
     } else if (ShouldSpecializeForDouble(ic_data)) {
       comp->set_operands_class_id(kDoubleCid);
     } else if (HasTwoMintOrSmi(*comp->ic_data()) &&
                FlowGraphCompiler::SupportsUnboxedMints()) {
       comp->set_operands_class_id(kMintCid);
     } else {
       ASSERT(comp->operands_class_id() == kIllegalCid);
     }
   } else if (HasTwoMintOrSmi(*comp->ic_data()) &&
              FlowGraphCompiler::SupportsUnboxedMints()) {
     comp->set_operands_class_id(kMintCid);
   }
 }
 
 
 void FlowGraphOptimizer::VisitRelationalOp(RelationalOpInstr* instr) {
-  HandleRelationalOp(this, instr, instr);
+  HandleRelationalOp(instr);
 }
 
 
 template <typename T>
-static void HandleEqualityCompare(FlowGraphOptimizer* optimizer,
-                                  EqualityCompareInstr* comp,
-                                  T instr,
-                                  ForwardInstructionIterator* iterator) {
+void FlowGraphOptimizer::HandleEqualityCompare(EqualityCompareInstr* comp,
+                                               T current_instruction) {

[Vyacheslav Egorov (Google), 2013/02/08 16:44:49]

I think this function is un-necessarily a template.

I also find BranchInstr::ReplaceWith method extremely confusing because it does
something that I would not expect from knowing Definition::ReplaceWith.

My suggestion here would be to remove templating of this function and make this
function return a replacement.

Then caller can handle replacement with either Definition::ReplaceWith or
BranchInstr::ReplaceComparisonWith.

It is also true that this is not in sync with HandleRelationalOp in terms of
passing in the current instruction.

[Kevin Millikin (Google), 2013/02/19 10:49:30]

I agree the architecture is a mess.  It's a preexisting mess that is, as you
say, confusing.  I'd rather not change it as part of this effort.

   // If one of the inputs is null, no ICdata will be collected.
   if (comp->left()->BindsToConstantNull() ||
       comp->right()->BindsToConstantNull()) {
     Token::Kind strict_kind = (comp->kind() == Token::kEQ) ?
         Token::kEQ_STRICT : Token::kNE_STRICT;
     StrictCompareInstr* strict_comp =
-        new StrictCompareInstr(strict_kind, comp->left(), comp->right());
-    instr->ReplaceWith(strict_comp, iterator);
+        new StrictCompareInstr(strict_kind,
+                               comp->left()->Copy(),
+                               comp->right()->Copy());
+    current_instruction->ReplaceWith(strict_comp, current_iterator());
     return;
   }
   if (!comp->HasICData() || (comp->ic_data()->NumberOfChecks() == 0)) {
     return;
   }
   ASSERT(comp->ic_data()->num_args_tested() == 2);
   if (comp->ic_data()->NumberOfChecks() == 1) {
     GrowableArray<intptr_t> class_ids;
     Function& target = Function::Handle();
     comp->ic_data()->GetCheckAt(0, &class_ids, &target);
     // TODO(srdjan): allow for mixed mode int/double comparison.
 
     if ((class_ids[0] == kSmiCid) && (class_ids[1] == kSmiCid)) {
-      optimizer->InsertBefore(
-          instr,
-          new CheckSmiInstr(comp->left()->Copy(), comp->deopt_id()),
-          instr->env(),
-          Definition::kEffect);
-      optimizer->InsertBefore(
-          instr,
-          new CheckSmiInstr(comp->right()->Copy(), comp->deopt_id()),
-          instr->env(),
-          Definition::kEffect);
+      InsertBefore(current_instruction,
+                   new CheckSmiInstr(comp->left()->Copy(), comp->deopt_id()),
+                   current_instruction->env(),
+                   Definition::kEffect);
+      InsertBefore(current_instruction,
+                   new CheckSmiInstr(comp->right()->Copy(), comp->deopt_id()),
+                   current_instruction->env(),
+                   Definition::kEffect);
       comp->set_receiver_class_id(kSmiCid);
     } else if ((class_ids[0] == kDoubleCid) && (class_ids[1] == kDoubleCid)) {
       comp->set_receiver_class_id(kDoubleCid);
     } else if (HasTwoMintOrSmi(*comp->ic_data()) &&
                FlowGraphCompiler::SupportsUnboxedMints()) {
       comp->set_receiver_class_id(kMintCid);
     } else {
       ASSERT(comp->receiver_class_id() == kIllegalCid);
     }
   } else if (HasTwoMintOrSmi(*comp->ic_data()) &&
              FlowGraphCompiler::SupportsUnboxedMints()) {
     comp->set_receiver_class_id(kMintCid);
   }
 
   if (comp->receiver_class_id() != kIllegalCid) {
     // Done.
     return;
   }
 
   // Check if ICDData contains checks with Smi/Null combinations. In that case
   // we can still emit the optimized Smi equality operation but need to add
   // checks for null or Smi.
   // TODO(srdjan): Add it for Double and Mint.
   GrowableArray<intptr_t> smi_or_null(2);
   smi_or_null.Add(kSmiCid);
   smi_or_null.Add(kNullCid);
-  if (ICDataHasOnlyReceiverArgumentClassIds(
-        *comp->ic_data(), smi_or_null, smi_or_null)) {
+  if (ICDataHasOnlyReceiverArgumentClassIds(*comp->ic_data(),
+                                            smi_or_null,
+                                            smi_or_null)) {
     const ICData& unary_checks_0 =
         ICData::ZoneHandle(comp->ic_data()->AsUnaryClassChecks());
-    const intptr_t deopt_id = comp->deopt_id();
-    if ((unary_checks_0.NumberOfChecks() == 1) &&
-        (unary_checks_0.GetReceiverClassIdAt(0) == kSmiCid)) {
-      // Smi only.
-      optimizer->InsertBefore(
-        instr,
-        new CheckSmiInstr(comp->left()->Copy(), deopt_id),
-        instr->env(),
-        Definition::kEffect);
-    } else {
-      // Smi or NULL.
-      optimizer->InsertBefore(
-        instr,
-        new CheckClassInstr(comp->left()->Copy(), deopt_id, unary_checks_0),
-        instr->env(),
-        Definition::kEffect);
-    }
+    AddCheckClass(comp->left()->definition(),
+                  unary_checks_0,
+                  comp->deopt_id(),
+                  current_instruction->env(),
+                  current_instruction);
 
     const ICData& unary_checks_1 =
         ICData::ZoneHandle(comp->ic_data()->AsUnaryClassChecksForArgNr(1));
-    if ((unary_checks_1.NumberOfChecks() == 1) &&
-        (unary_checks_1.GetReceiverClassIdAt(0) == kSmiCid)) {
-      // Smi only.
-      optimizer->InsertBefore(
-        instr,
-        new CheckSmiInstr(comp->right()->Copy(), deopt_id),
-        instr->env(),
-        Definition::kEffect);
-    } else {
-      // Smi or NULL.
-      optimizer->InsertBefore(
-        instr,
-        new CheckClassInstr(comp->right()->Copy(), deopt_id, unary_checks_1),
-        instr->env(),
-        Definition::kEffect);
-    }
+    AddCheckClass(comp->right()->definition(),
+                  unary_checks_1,
+                  comp->deopt_id(),
+                  current_instruction->env(),
+                  current_instruction);
     comp->set_receiver_class_id(kSmiCid);
   }
 }
 
 
 void FlowGraphOptimizer::VisitEqualityCompare(EqualityCompareInstr* instr) {
-  HandleEqualityCompare(this, instr, instr, current_iterator());
+  HandleEqualityCompare(instr, instr);
 }
 
 
 void FlowGraphOptimizer::VisitBranch(BranchInstr* instr) {
   ComparisonInstr* comparison = instr->comparison();
   if (comparison->IsRelationalOp()) {
-    HandleRelationalOp(this, comparison->AsRelationalOp(), instr);
+    HandleRelationalOp(comparison->AsRelationalOp());
   } else if (comparison->IsEqualityCompare()) {
-    HandleEqualityCompare(this, comparison->AsEqualityCompare(), instr,
-                          current_iterator());
+    HandleEqualityCompare(comparison->AsEqualityCompare(), instr);
   } else {
     ASSERT(comparison->IsStrictCompare());
     // Nothing to do.
   }
 }
 
 
 static bool MayBeBoxableNumber(intptr_t cid) {
   return (cid == kDynamicCid) ||
          (cid == kMintCid) ||
@@ skipping 445 matching lines @@
   }
 }
 
 
 ConstraintInstr* RangeAnalysis::InsertConstraintFor(Definition* defn,
                                                     Range* constraint_range,
                                                     Instruction* after) {
   // No need to constrain constants.
   if (defn->IsConstant()) return NULL;
 
-  Value* value = new Value(defn);
-  ConstraintInstr* constraint = new ConstraintInstr(value, constraint_range);
-  constraint->InsertAfter(after);
-  constraint->set_ssa_temp_index(flow_graph_->alloc_ssa_temp_index());
-  RenameDominatedUses(defn, after, constraint);
+  ConstraintInstr* constraint =
+      new ConstraintInstr(new Value(defn), constraint_range);
+  flow_graph_->InsertAfter(after, constraint, NULL, Definition::kValue);
+  RenameDominatedUses(defn, constraint, constraint);

[Vyacheslav Egorov (Google), 2013/02/08 16:44:49]

That was an easy fix. Nice!

   constraints_.Add(constraint);
-  value->set_instruction(constraint);
-  value->set_use_index(0);
-  defn->AddInputUse(value);
   return constraint;
 }
 
 
 void RangeAnalysis::ConstrainValueAfterBranch(Definition* defn, Value* use) {
   BranchInstr* branch = use->instruction()->AsBranch();
   RelationalOpInstr* rel_op = branch->comparison()->AsRelationalOp();
   if ((rel_op != NULL) && (rel_op->operands_class_id() == kSmiCid)) {
     // Found comparison of two smis. Constrain defn at true and false
     // successors using the other operand as a boundary.
@@ skipping 413 matching lines @@
 
 void FlowGraphTypePropagator::VisitInstanceOf(InstanceOfInstr* instr) {
   bool is_null;
   bool is_instance = false;
   if (FLAG_eliminate_type_checks &&
       instr->value()->CanComputeIsNull(&is_null) &&
       (is_null ||
        instr->value()->CanComputeIsInstanceOf(instr->type(), &is_instance))) {
     bool val = instr->negate_result() ? !is_instance : is_instance;
     Definition* result = new ConstantInstr(val ? Bool::True() : Bool::False());
-    result->set_ssa_temp_index(flow_graph_->alloc_ssa_temp_index());
-    result->InsertBefore(instr);
-    // Replace uses and remove the current instruction via the iterator.
-    instr->ReplaceUsesWith(result);
-    ASSERT(current_iterator()->Current() == instr);
-    current_iterator()->RemoveCurrentFromGraph();
+    instr->ReplaceWith(result, current_iterator());
     if (FLAG_trace_optimization) {
       OS::Print("Replacing v%"Pd" with v%"Pd"\n",
                 instr->ssa_temp_index(),
                 result->ssa_temp_index());
     }
 
     if (FLAG_trace_type_check_elimination) {
       FlowGraphPrinter::PrintTypeCheck(parsed_function(),
                                        instr->token_pos(),
                                        instr->value(),
@@ skipping 1816 matching lines @@
 
   if (FLAG_trace_constant_propagation) {
     OS::Print("\n==== After constant propagation ====\n");
     FlowGraphPrinter printer(*graph_);
     printer.PrintBlocks();
   }
 }
 
 
 }  // namespace dart