VM: [AOT] Make sure that we inline all resolved accessor invocations.

runtime/vm/aot_optimizer.cc

Index: runtime/vm/aot_optimizer.cc
diff --git a/runtime/vm/aot_optimizer.cc b/runtime/vm/aot_optimizer.cc
index 025784e6fb74c927addba3d9e8c984b203d0dc8b..5d981eb0a19c0a2a8c1734ff27b7aec6ef73fd31 100644
--- a/runtime/vm/aot_optimizer.cc
+++ b/runtime/vm/aot_optimizer.cc
@@ -738,18 +738,6 @@ static bool ArgIsAlways(intptr_t cid,
 }
 
 
-bool AotOptimizer::TryReplaceWithIndexedOp(InstanceCallInstr* call) {

[Florian Schneider, 2016/06/24 16:09:58]

I'd keep this code around:

FlowGraphInliner::TryInlineRecognizedMethod should inline certain methods even
in AOT.

It does not do it now - but that seems like an oversight.

Also by moving some of this code into FlowGraphInliner, we can avoid duplication
between jit_optimizer.cc and aot_optimizer.cc.

[Vyacheslav Egorov (Google), 2016/06/24 16:13:04]

I think it does not make much sense to keep around the code that does not do
anything right now - it's just called and does essentially nothing (just
allocates some zone memory :))

Also as you say: when we actually start enabling it we should strive to unify it
as much as possible with the JIT - which means that this code will not survive
in its current form anyway. 

[Florian Schneider, 2016/06/24 22:48:17]

We should make TryInlineRecognizedMethod work with the AOT optimizer. I can look
into that. I think it was just forgotten to enable this code...

-  // Check for monomorphic IC data.
-  if (!call->HasICData()) return false;
-  const ICData& ic_data =
-      ICData::Handle(Z, call->ic_data()->AsUnaryClassChecks());
-  if (ic_data.NumberOfChecks() != 1) {
-    return false;
-  }
-  return TryReplaceInstanceCallWithInline(call);
-}
-
-
 // Return true if d is a string of length one (a constant or result from
 // from string-from-char-code instruction.
 static bool IsLengthOneString(Definition* d) {
@@ -1629,50 +1617,6 @@ bool AotOptimizer::TryInlineInstanceGetter(InstanceCallInstr* call) {
 }
 
 
-bool AotOptimizer::TryReplaceInstanceCallWithInline(
-    InstanceCallInstr* call) {
-  Function& target = Function::Handle(Z);
-  GrowableArray<intptr_t> class_ids;
-  call->ic_data()->GetCheckAt(0, &class_ids, &target);
-  const intptr_t receiver_cid = class_ids[0];
-
-  TargetEntryInstr* entry;
-  Definition* last;
-  if (!FlowGraphInliner::TryInlineRecognizedMethod(flow_graph_,
-                                                   receiver_cid,
-                                                   target,
-                                                   call,
-                                                   call->ArgumentAt(0),
-                                                   call->token_pos(),
-                                                   *call->ic_data(),
-                                                   &entry, &last)) {
-    return false;
-  }
-
-  // Insert receiver class check.
-  AddReceiverCheck(call);
-  // Remove the original push arguments.
-  for (intptr_t i = 0; i < call->ArgumentCount(); ++i) {
-    PushArgumentInstr* push = call->PushArgumentAt(i);
-    push->ReplaceUsesWith(push->value()->definition());
-    push->RemoveFromGraph();
-  }
-  // Replace all uses of this definition with the result.
-  call->ReplaceUsesWith(last);
-  // Finally insert the sequence other definition in place of this one in the
-  // graph.
-  call->previous()->LinkTo(entry->next());
-  entry->UnuseAllInputs();  // Entry block is not in the graph.
-  last->LinkTo(call);
-  // Remove through the iterator.
-  ASSERT(current_iterator()->Current() == call);
-  current_iterator()->RemoveCurrentFromGraph();
-  call->set_previous(NULL);
-  call->set_next(NULL);
-  return true;
-}
-
-
 void AotOptimizer::ReplaceWithMathCFunction(
     InstanceCallInstr* call,
     MethodRecognizer::Kind recognized_kind) {
@@ -1691,210 +1635,6 @@ void AotOptimizer::ReplaceWithMathCFunction(
 }
 
 
-static bool IsSupportedByteArrayViewCid(intptr_t cid) {
-  switch (cid) {
-    case kTypedDataInt8ArrayCid:
-    case kTypedDataUint8ArrayCid:
-    case kExternalTypedDataUint8ArrayCid:
-    case kTypedDataUint8ClampedArrayCid:
-    case kExternalTypedDataUint8ClampedArrayCid:
-    case kTypedDataInt16ArrayCid:
-    case kTypedDataUint16ArrayCid:
-    case kTypedDataInt32ArrayCid:
-    case kTypedDataUint32ArrayCid:
-    case kTypedDataFloat32ArrayCid:
-    case kTypedDataFloat64ArrayCid:
-    case kTypedDataFloat32x4ArrayCid:
-    case kTypedDataInt32x4ArrayCid:
-      return true;
-    default:
-      return false;
-  }
-}
-
-
-// Inline only simple, frequently called core library methods.
-bool AotOptimizer::TryInlineInstanceMethod(InstanceCallInstr* call) {
-  ASSERT(call->HasICData());
-  const ICData& ic_data = *call->ic_data();
-  if ((ic_data.NumberOfUsedChecks() == 0) || !ic_data.HasOneTarget()) {
-    // No type feedback collected or multiple targets found.
-    return false;
-  }
-
-  Function& target = Function::Handle(Z);
-  GrowableArray<intptr_t> class_ids;
-  ic_data.GetCheckAt(0, &class_ids, &target);
-  MethodRecognizer::Kind recognized_kind =
-      MethodRecognizer::RecognizeKind(target);
-
-  if ((recognized_kind == MethodRecognizer::kGrowableArraySetData) &&
-      (ic_data.NumberOfChecks() == 1) &&
-      (class_ids[0] == kGrowableObjectArrayCid)) {
-    // This is an internal method, no need to check argument types.
-    Definition* array = call->ArgumentAt(0);
-    Definition* value = call->ArgumentAt(1);
-    StoreInstanceFieldInstr* store = new(Z) StoreInstanceFieldInstr(
-        GrowableObjectArray::data_offset(),
-        new(Z) Value(array),
-        new(Z) Value(value),
-        kEmitStoreBarrier,
-        call->token_pos());
-    ReplaceCall(call, store);
-    return true;
-  }
-
-  if ((recognized_kind == MethodRecognizer::kGrowableArraySetLength) &&
-      (ic_data.NumberOfChecks() == 1) &&
-      (class_ids[0] == kGrowableObjectArrayCid)) {
-    // This is an internal method, no need to check argument types nor
-    // range.
-    Definition* array = call->ArgumentAt(0);
-    Definition* value = call->ArgumentAt(1);
-    StoreInstanceFieldInstr* store = new(Z) StoreInstanceFieldInstr(
-        GrowableObjectArray::length_offset(),
-        new(Z) Value(array),
-        new(Z) Value(value),
-        kNoStoreBarrier,
-        call->token_pos());
-    ReplaceCall(call, store);
-    return true;
-  }
-
-  if ((recognized_kind == MethodRecognizer::kOneByteStringCodeUnitAt) ||
-      (recognized_kind == MethodRecognizer::kTwoByteStringCodeUnitAt) ||
-      (recognized_kind == MethodRecognizer::kExternalOneByteStringCodeUnitAt) ||
-      (recognized_kind == MethodRecognizer::kExternalTwoByteStringCodeUnitAt)) {
-      ASSERT(ic_data.NumberOfChecks() == 1);
-      ASSERT((class_ids[0] == kOneByteStringCid) ||
-             (class_ids[0] == kTwoByteStringCid) ||
-             (class_ids[0] == kExternalOneByteStringCid) ||
-             (class_ids[0] == kExternalTwoByteStringCid));
-    return TryReplaceInstanceCallWithInline(call);
-  }
-
-  if ((recognized_kind == MethodRecognizer::kStringBaseCharAt) &&
-      (ic_data.NumberOfChecks() == 1)) {
-      ASSERT((class_ids[0] == kOneByteStringCid) ||
-             (class_ids[0] == kTwoByteStringCid) ||
-             (class_ids[0] == kExternalOneByteStringCid) ||
-             (class_ids[0] == kExternalTwoByteStringCid));
-    return TryReplaceInstanceCallWithInline(call);
-  }
-
-  if ((class_ids[0] == kOneByteStringCid) && (ic_data.NumberOfChecks() == 1)) {
-    if (recognized_kind == MethodRecognizer::kOneByteStringSetAt) {
-      // This is an internal method, no need to check argument types nor
-      // range.
-      Definition* str = call->ArgumentAt(0);
-      Definition* index = call->ArgumentAt(1);
-      Definition* value = call->ArgumentAt(2);
-      StoreIndexedInstr* store_op = new(Z) StoreIndexedInstr(
-          new(Z) Value(str),
-          new(Z) Value(index),
-          new(Z) Value(value),
-          kNoStoreBarrier,
-          1,  // Index scale
-          kOneByteStringCid,
-          call->deopt_id(),
-          call->token_pos());
-      ReplaceCall(call, store_op);
-      return true;
-    }
-    return false;
-  }
-
-  if (CanUnboxDouble() &&
-      (recognized_kind == MethodRecognizer::kIntegerToDouble) &&
-      (ic_data.NumberOfChecks() == 1)) {
-    if (class_ids[0] == kSmiCid) {
-      AddReceiverCheck(call);
-      ReplaceCall(call,
-                  new(Z) SmiToDoubleInstr(
-                      new(Z) Value(call->ArgumentAt(0)),
-                      call->token_pos()));
-      return true;
-    } else if ((class_ids[0] == kMintCid) && CanConvertUnboxedMintToDouble()) {
-      AddReceiverCheck(call);
-      ReplaceCall(call,
-                  new(Z) MintToDoubleInstr(new(Z) Value(call->ArgumentAt(0)),
-                                           call->deopt_id()));
-      return true;
-    }
-  }
-
-  if (class_ids[0] == kDoubleCid) {
-    if (!CanUnboxDouble()) {
-      return false;
-    }
-    switch (recognized_kind) {
-      case MethodRecognizer::kDoubleToInteger: {
-        AddReceiverCheck(call);
-        ASSERT(call->HasICData());
-        const ICData& ic_data = *call->ic_data();
-        Definition* input = call->ArgumentAt(0);
-        Definition* d2i_instr = NULL;
-        if (ic_data.HasDeoptReason(ICData::kDeoptDoubleToSmi)) {
-          // Do not repeatedly deoptimize because result didn't fit into Smi.
-          d2i_instr =  new(Z) DoubleToIntegerInstr(
-              new(Z) Value(input), call);
-        } else {
-          // Optimistically assume result fits into Smi.
-          d2i_instr = new(Z) DoubleToSmiInstr(
-              new(Z) Value(input), call->deopt_id());
-        }
-        ReplaceCall(call, d2i_instr);
-        return true;
-      }
-      case MethodRecognizer::kDoubleMod:
-      case MethodRecognizer::kDoubleRound:
-        ReplaceWithMathCFunction(call, recognized_kind);
-        return true;
-      case MethodRecognizer::kDoubleTruncate:
-      case MethodRecognizer::kDoubleFloor:
-      case MethodRecognizer::kDoubleCeil:
-        if (!TargetCPUFeatures::double_truncate_round_supported()) {
-          ReplaceWithMathCFunction(call, recognized_kind);
-        } else {
-          AddReceiverCheck(call);
-          DoubleToDoubleInstr* d2d_instr =
-              new(Z) DoubleToDoubleInstr(new(Z) Value(call->ArgumentAt(0)),
-                                         recognized_kind, call->deopt_id());
-          ReplaceCall(call, d2d_instr);
-        }
-        return true;
-      case MethodRecognizer::kDoubleAdd:
-      case MethodRecognizer::kDoubleSub:
-      case MethodRecognizer::kDoubleMul:
-      case MethodRecognizer::kDoubleDiv:
-        return TryReplaceInstanceCallWithInline(call);
-      default:
-        // Unsupported method.
-        return false;
-    }
-  }
-
-  if (IsSupportedByteArrayViewCid(class_ids[0]) &&
-      (ic_data.NumberOfChecks() == 1)) {
-    return TryReplaceInstanceCallWithInline(call);
-  }
-
-  if ((class_ids[0] == kFloat32x4Cid) && (ic_data.NumberOfChecks() == 1)) {
-    return TryInlineFloat32x4Method(call, recognized_kind);
-  }
-
-  if ((class_ids[0] == kInt32x4Cid) && (ic_data.NumberOfChecks() == 1)) {
-    return TryInlineInt32x4Method(call, recognized_kind);
-  }
-
-  if ((class_ids[0] == kFloat64x2Cid) && (ic_data.NumberOfChecks() == 1)) {
-    return TryInlineFloat64x2Method(call, recognized_kind);
-  }
-
-  return false;
-}
-
-
 bool AotOptimizer::TryInlineFloat32x4Constructor(
     StaticCallInstr* call,
     MethodRecognizer::Kind recognized_kind) {
@@ -2292,6 +2032,24 @@ static bool HasLikelySmiOperand(InstanceCallInstr* instr) {
 }
 
 
+bool AotOptimizer::TryInlineFieldAccess(InstanceCallInstr* call) {
+  const Token::Kind op_kind = call->token_kind();
+  if ((op_kind == Token::kGET) && TryInlineInstanceGetter(call)) {
+    return true;
+  }
+
+  const ICData& unary_checks =
+      ICData::Handle(Z, call->ic_data()->AsUnaryClassChecks());
+  if ((unary_checks.NumberOfChecks() > 0) &&
+      (op_kind == Token::kSET) &&
+      TryInlineInstanceSetter(call, unary_checks)) {
+    return true;
+  }
+
+  return false;
+}
+
+
 // Tries to optimize instance call by replacing it with a faster instruction
 // (e.g, binary op, field load, ..).
 void AotOptimizer::VisitInstanceCall(InstanceCallInstr* instr) {
@@ -2310,26 +2068,14 @@ void AotOptimizer::VisitInstanceCall(InstanceCallInstr* instr) {
     return;
   }
 
-  if ((op_kind == Token::kGET) &&
-      TryInlineInstanceGetter(instr)) {
+  if (TryInlineFieldAccess(instr)) {
     return;
   }
+
   const ICData& unary_checks =
       ICData::ZoneHandle(Z, instr->ic_data()->AsUnaryClassChecks());
-  if ((unary_checks.NumberOfChecks() > 0) &&
-      (op_kind == Token::kSET) &&
-      TryInlineInstanceSetter(instr, unary_checks)) {
-    return;
-  }
-
   if (IsAllowedForInlining(instr->deopt_id()) &&
       (unary_checks.NumberOfChecks() > 0)) {
-    if ((op_kind == Token::kINDEX) && TryReplaceWithIndexedOp(instr)) {
-      return;
-    }
-    if ((op_kind == Token::kASSIGN_INDEX) && TryReplaceWithIndexedOp(instr)) {
-      return;
-    }
     if ((op_kind == Token::kEQ) && TryReplaceWithEqualityOp(instr, op_kind)) {
       return;
     }
@@ -2513,6 +2259,26 @@ void AotOptimizer::VisitInstanceCall(InstanceCallInstr* instr) {
       }
 
       if (single_target.raw() != Function::null()) {
+        // If this is a getter or setter invocation try inlining it right away
+        // instead of replacing it with a static call.
+        if ((op_kind == Token::kGET) || (op_kind == Token::kSET)) {
+          // Create fake IC data with the resolved target.
+          const ICData& ic_data = ICData::Handle(
+              ICData::New(flow_graph_->function(),
+                          instr->function_name(),
+                          args_desc_array,
+                          Thread::kNoDeoptId,
+                          /* args_tested = */ 1,
+                          false));
+          cls = single_target.Owner();
+          ic_data.AddReceiverCheck(cls.id(), single_target);
+          instr->set_ic_data(&ic_data);
+
+          if (TryInlineFieldAccess(instr)) {
+            return;
+          }
+        }
+
         // We have computed that there is only a single target for this call
         // within the whole hierarchy. Replace InstanceCall with StaticCall.
         ZoneGrowableArray<PushArgumentInstr*>* args =