Inline indexed store ([]=) array operations in checked mode.

runtime/vm/flow_graph_optimizer.cc

Index: runtime/vm/flow_graph_optimizer.cc
===================================================================
--- runtime/vm/flow_graph_optimizer.cc	(revision 13776)
+++ runtime/vm/flow_graph_optimizer.cc	(working copy)
@@ -418,8 +418,6 @@
 
 
 bool FlowGraphOptimizer::TryReplaceWithStoreIndexed(InstanceCallInstr* call) {
-  // TODO(fschneider): Optimize []= operator in checked mode as well.
-  if (FLAG_enable_type_checks) return false;
   const intptr_t class_id = ReceiverClassId(call);
   ICData& value_check = ICData::Handle();
   switch (class_id) {
@@ -439,6 +437,69 @@
     default:
       return false;
   }
+
+  if (FLAG_enable_type_checks) {
+    Value* array = call->ArgumentAt(0)->value();
+    Value* index = call->ArgumentAt(1)->value();
+    Value* value = call->ArgumentAt(2)->value();
+    const Function& target =
+        Function::ZoneHandle(call->ic_data()->GetTargetAt(0));
+
+    // Type check for the index.
+    const AbstractType& index_type =
+        AbstractType::ZoneHandle(target.ParameterTypeAt(1));
+    ASSERT(index_type.IsIntType());
+    ASSERT(index_type.IsInstantiated());
+    ConstantInstr* null_constant = new ConstantInstr(Object::ZoneHandle());
+    InsertBefore(call, null_constant, NULL, Definition::kValue);
+    AssertAssignableInstr* assert_index =
+        new AssertAssignableInstr(call->token_pos(),
+                                  index->Copy(),
+                                  new Value(null_constant),
+                                  new Value(null_constant),
+                                  index_type,
+                                  String::ZoneHandle(Symbols::New("index")));
+    InsertBefore(call, assert_index, NULL, Definition::kValue);

[srdjan, 2012/10/18 16:25:48]

Do we need the index check at all, since it will be done as part of StoreIndexed
code. (deoptimizes if index is not integer).

[Florian Schneider, 2012/10/18 18:13:10]

Good point. I agree that we don't need it here. The unoptimized code will take
care of throwing the proper exception in case the index is not a smi.

+
+    // Type check for the value.
+    const AbstractType& value_type =
+        AbstractType::ZoneHandle(target.ParameterTypeAt(2));
+    Value* instantiator = NULL;
+    Value* type_args = NULL;
+    switch (class_id) {
+      case kArrayCid:
+      case kGrowableObjectArrayCid: {
+        const Class& instantiator_class = Class::Handle(target.Owner());
+        intptr_t type_arguments_instance_field_offset =
+            instantiator_class.type_arguments_instance_field_offset();
+        LoadFieldInstr* load_type_args =
+            new LoadFieldInstr(array->Copy(),
+                               type_arguments_instance_field_offset,
+                               Type::ZoneHandle());  // No type.
+        InsertBefore(call, load_type_args, NULL, Definition::kValue);
+        instantiator = array->Copy();
+        type_args = new Value(load_type_args);
+        break;
+      }
+      case kFloat64ArrayCid:
+        instantiator = new Value(null_constant);
+        type_args = new Value(null_constant);
+        ASSERT(value_type.IsDoubleType());
+        ASSERT(value_type.IsInstantiated());
+        break;
+      default:
+        UNREACHABLE();

[regis, 2012/10/18 17:07:26]

How about a comment explaining that other array flavors are not inlined yet?

[Florian Schneider, 2012/10/18 18:13:10]

Done.

+    }
+    AssertAssignableInstr* assert_value =
+        new AssertAssignableInstr(call->token_pos(),
+                                  value->Copy(),
+                                  instantiator,
+                                  type_args,
+                                  value_type,
+                                  String::ZoneHandle(Symbols::New("value")));
+    InsertBefore(call, assert_value, NULL, Definition::kValue);
+  }
+
   Value* array = NULL;
   Value* index = NULL;
   intptr_t array_cid = PrepareIndexedOp(call, class_id, &array, &index);