Enable polymorphic inlining of StringBase [] and StringBase codeUnitAt

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 1343 matching lines @@
     case MethodRecognizer::kByteArrayBaseGetFloat32x4:
       if (!ShouldInlineSimd()) return false;
       return InlineByteArrayViewLoad(call, receiver, receiver_cid,
                                      kTypedDataFloat32x4ArrayCid,
                                      ic_data, entry, last);
     case MethodRecognizer::kByteArrayBaseGetInt32x4:
       if (!ShouldInlineSimd()) return false;
       return InlineByteArrayViewLoad(call, receiver, receiver_cid,
                                      kTypedDataInt32x4ArrayCid,
                                      ic_data, entry, last);
+    case MethodRecognizer::kStringBaseCodeUnitAt:
+      return InlineStringCodeUnitAt(call, receiver_cid, entry, last);
+    case MethodRecognizer::kStringBaseCharAt:
+      return InlineStringBaseCharAt(call, receiver_cid, entry, last);
     default:
       return false;
   }
 }
 
 
 intptr_t FlowGraphOptimizer::PrepareInlineIndexedOp(Instruction* call,
                                                     intptr_t array_cid,
                                                     Definition** array,
                                                     Definition* index,
@@ skipping 988 matching lines @@
   if (target.kind() != RawFunction::kImplicitGetter) {
     // Non-implicit getters are inlined like normal methods by conventional
     // inlining in FlowGraphInliner.
     return false;
   }
   InlineImplicitInstanceGetter(call);
   return true;
 }
 
 
+// Returns the LoadIndexedInstr.
+Definition* FlowGraphOptimizer::PrepareInlineStringIndexOp(
+    Instruction* call,
+    intptr_t cid,
+    Definition* str,
+    Definition* index,
+    Instruction* cursor) {
+
+  cursor = flow_graph()->AppendTo(cursor,
+                                  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;

[Florian Schneider, 2014/02/13 11:57:46]

I'm pretty sure that constant propagation handles bounds checks properly by now.
You can remove the skip_check variable and the code below and always emit the
bounds check. It will be eliminated later on.

+  if (str->IsConstant() && index->IsConstant()) {
+    const String& constant_string =
+        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) {
+    // Load the length of the string.
+    LoadFieldInstr* length = BuildLoadStringLength(str);
+    cursor = flow_graph()->AppendTo(cursor, length, NULL, Definition::kValue);
+    // Bounds check.
+    cursor = flow_graph()->AppendTo(cursor,
+                                     new CheckArrayBoundInstr(new Value(length),
+                                                              new Value(index),
+                                                              call->deopt_id()),
+                                     call->env(),
+                                     Definition::kEffect);
+  }
+  LoadIndexedInstr* load_indexed = new LoadIndexedInstr(
+      new Value(str),
+      new Value(index),
+      FlowGraphCompiler::ElementSizeFor(cid),
+      cid,
+      Isolate::kNoDeoptId);
+
+  cursor = flow_graph()->AppendTo(cursor,
+                                  load_indexed,
+                                  NULL,
+                                  Definition::kValue);
+  ASSERT(cursor == load_indexed);
+  return load_indexed;
+}
+
+
+bool FlowGraphOptimizer::InlineStringCodeUnitAt(
+    Instruction* call,
+    intptr_t cid,
+    TargetEntryInstr** entry,
+    Definition** last) {
+
+  Definition* str = call->ArgumentAt(0);
+  Definition* index = call->ArgumentAt(1);
+
+  *entry = new TargetEntryInstr(flow_graph()->allocate_block_id(),
+                                call->GetBlock()->try_index());
+  (*entry)->InheritDeoptTarget(call);
+
+  *last = PrepareInlineStringIndexOp(call, cid, str, index, *entry);
+
+  return true;
+}
+
+
+bool FlowGraphOptimizer::InlineStringBaseCharAt(
+    Instruction* call,
+    intptr_t cid,
+    TargetEntryInstr** entry,
+    Definition** last) {
+
+  Definition* str = call->ArgumentAt(0);
+  Definition* index = call->ArgumentAt(1);
+
+  *entry = new TargetEntryInstr(flow_graph()->allocate_block_id(),
+                                call->GetBlock()->try_index());
+  (*entry)->InheritDeoptTarget(call);
+
+  *last = PrepareInlineStringIndexOp(call, cid, str, index, *entry);
+
+  StringFromCharCodeInstr* char_at =
+          new StringFromCharCodeInstr(new Value(*last), cid);
+
+  flow_graph()->AppendTo(*last, char_at, NULL, Definition::kValue);
+  *last = char_at;
+
+  return true;
+}
+
+
 LoadIndexedInstr* FlowGraphOptimizer::BuildStringCodeUnitAt(

[Florian Schneider, 2014/02/13 11:57:46]

I think you should be able to use your newly introduced builder function instead
of this one.

     InstanceCallInstr* call,
     intptr_t cid) {
   Definition* str = call->ArgumentAt(0);
   Definition* index = call->ArgumentAt(1);
   AddReceiverCheck(call);
   InsertBefore(call,
                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.
@@ skipping 109 matching lines @@
   }
 
   if ((recognized_kind == MethodRecognizer::kStringBaseCodeUnitAt) &&
       (ic_data.NumberOfChecks() == 1) &&
       ((class_ids[0] == kOneByteStringCid) ||
        (class_ids[0] == kTwoByteStringCid))) {
     LoadIndexedInstr* instr = BuildStringCodeUnitAt(call, class_ids[0]);
     ReplaceCall(call, instr);
     return true;
   }
   if ((class_ids[0] == kOneByteStringCid) && (ic_data.NumberOfChecks() == 1)) {

[Florian Schneider, 2014/02/13 11:57:46]

Here you should use the newly added builder functions instead of the old ones.
As as positive side-effect you also get 2-byte strings handled for free.

     if (recognized_kind == MethodRecognizer::kStringBaseCharAt) {
       // TODO(fschneider): Handle TwoByteString.
       LoadIndexedInstr* load_char_code =
           BuildStringCodeUnitAt(call, class_ids[0]);
       InsertBefore(call, load_char_code, NULL, Definition::kValue);
       StringFromCharCodeInstr* char_at =
           new StringFromCharCodeInstr(new Value(load_char_code),
                                       kOneByteStringCid);
       ReplaceCall(call, char_at);
       return true;
@@ skipping 5983 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