[vm/corelib] Remove GrowableArrayMarker hack.

GrowableArrayMarker was a class that implemented int and was used to
enable implementation of default List factory constructor in pure Dart:

  factory List([int length = GROWABLE_ARRAY_MARKER]) {
    return identical(length, GROWABLE_ARRAY_MARKER) ? new _GrowableList<E>(0)
                                                    : new _List<E>(length);
  }

Its existence complicated all kinds of things in the VM and it is finally
time to remove it.

Instead we build List factory body directly in IL.

This CL also provides inlining rule for `new List(n)` case.

Change-Id: I870751658a4ac17fce649c9ac70395ff88a5436c
Reviewed-on: https://dart-review.googlesource.com/57262
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>

pkg/vm/testcases/transformations/type_flow/transformer/bench_vector.dart.expect

@@ -17,7 +17,7 @@ class _Vector extends core::Object {
   }
   operator *([@vm.inferred-type.metadata=#lib::_Vector?] self::_Vector a) → core::double {
     core::double result = 0.0;
-    for (core::int i = 0; [@vm.inferred-type.metadata=dart.core::bool?] i.{core::num::<}([@vm.direct-call.metadata=#lib::_Vector::_length] [@vm.inferred-type.metadata=!] this.{self::_Vector::_length}); i = i.{core::num::+}(1))
+    for (core::int i = 0; [@vm.direct-call.metadata=dart.core::_IntegerImplementation::<??] [@vm.inferred-type.metadata=dart.core::bool?] i.{core::num::<}([@vm.direct-call.metadata=#lib::_Vector::_length] [@vm.inferred-type.metadata=!] this.{self::_Vector::_length}); i = i.{core::num::+}(1))
       result = [@vm.direct-call.metadata=dart.core::_Double::+??] [@vm.inferred-type.metadata=dart.core::_Double] result.{core::double::+}([@vm.direct-call.metadata=dart.core::_Double::*] [@vm.inferred-type.metadata=dart.core::_Double] [@vm.direct-call.metadata=#lib::_Vector::[]] [@vm.inferred-type.metadata=dart.core::_Double] this.{self::_Vector::[]}(i).{core::double::*}([@vm.direct-call.metadata=#lib::_Vector::[]??] [@vm.inferred-type.metadata=dart.core::_Double] a.{self::_Vector::[]}(i)));
     return result;
   }
@@ -26,7 +26,7 @@ class _Vector extends core::Object {
 [@vm.inferred-type.metadata=dart.core::_Double?]static field core::double x = 0.0;
 static method main(core::List<core::String> args) → dynamic {
   core::Stopwatch timer = let final core::Stopwatch #t4 = new core::Stopwatch::•() in let final dynamic #t5 = [@vm.direct-call.metadata=dart.core::Stopwatch::start] #t4.{core::Stopwatch::start}() in #t4;
-  for (core::int i = 0; [@vm.inferred-type.metadata=dart.core::bool?] i.{core::num::<}(100000000); i = i.{core::num::+}(1)) {
+  for (core::int i = 0; [@vm.direct-call.metadata=dart.core::_IntegerImplementation::<??] [@vm.inferred-type.metadata=dart.core::bool?] i.{core::num::<}(100000000); i = i.{core::num::+}(1)) {
     self::x = [@vm.direct-call.metadata=dart.core::_Double::+??] [@vm.inferred-type.metadata=dart.core::_Double] [@vm.inferred-type.metadata=dart.core::_Double?] self::x.{core::double::+}([@vm.direct-call.metadata=#lib::_Vector::*??] [@vm.inferred-type.metadata=dart.core::_Double] [@vm.inferred-type.metadata=#lib::_Vector?] self::v.{self::_Vector::*}([@vm.inferred-type.metadata=#lib::_Vector?] self::v));
   }
   [@vm.direct-call.metadata=dart.core::Stopwatch::stop] timer.{core::Stopwatch::stop}();

runtime/lib/array.cc

@@ -10,6 +10,12 @@
 
 namespace dart {
 
+DEFINE_NATIVE_ENTRY(List_new, 2) {
+  // This function is handled by flow-graph builder.
+  UNREACHABLE();
+  return Object::null();
+}
+
 DEFINE_NATIVE_ENTRY(List_allocate, 2) {
   // Implemented in FlowGraphBuilder::VisitNativeBody.
   UNREACHABLE();

runtime/lib/array_patch.dart

@@ -4,28 +4,10 @@
 
 // part of "core_patch.dart";
 
-// The _GrowableArrayMarker class is used to signal to the List() factory
-// whether a parameter was passed.
-class _GrowableArrayMarker implements int {
-  const _GrowableArrayMarker();
-  noSuchMethod(_) {
-    throw new UnimplementedError();
-  }
-}
-
-const _GROWABLE_ARRAY_MARKER = const _GrowableArrayMarker();
-
 @patch
 class List<E> {
   @patch
-  factory List([int length = _GROWABLE_ARRAY_MARKER]) {
-    if (identical(length, _GROWABLE_ARRAY_MARKER)) {
-      return new _GrowableList<E>(0);
-    }
-    // All error handling on the length parameter is done at the implementation
-    // of new _List.
-    return new _List<E>(length);
-  }
+  factory List([int length]) native "List_new";
 
   @patch
   factory List.filled(int length, E fill, {bool growable: false}) {

runtime/vm/bootstrap_natives.h

@@ -111,6 +111,7 @@ namespace dart {
   V(RegExp_getGroupCount, 1)                                                   \
   V(RegExp_ExecuteMatch, 3)                                                    \
   V(RegExp_ExecuteMatchSticky, 3)                                              \
+  V(List_new, 2)                                                               \
   V(List_allocate, 2)                                                          \
   V(List_getIndexed, 2)                                                        \
   V(List_setIndexed, 3)                                                        \

runtime/vm/compiler/backend/inliner.cc

@@ -3502,6 +3502,28 @@ bool FlowGraphInliner::TryInlineRecognizedMethod(
       return true;
     }
 
+    case MethodRecognizer::kListFactory: {
+      // We only want to inline new List(n) which decreases code size and
+      // improves performance. We don't want to inline new List().
+      if (call->ArgumentCount() != 2) {
+        return false;
+      }
+
+      const auto type = new (Z) Value(call->ArgumentAt(0));
+      const auto num_elements = new (Z) Value(call->ArgumentAt(1));
+      *entry = new (Z)
+          TargetEntryInstr(flow_graph->allocate_block_id(),
+                           call->GetBlock()->try_index(), Thread::kNoDeoptId);
+      (*entry)->InheritDeoptTarget(Z, call);
+      *last = new (Z) CreateArrayInstr(call->token_pos(), type, num_elements,
+                                       call->deopt_id());
+      flow_graph->AppendTo(
+          *entry, *last,
+          call->deopt_id() != Thread::kNoDeoptId ? call->env() : NULL,
+          FlowGraph::kValue);
+      return true;
+    }
+
     case MethodRecognizer::kObjectArrayAllocate: {
       Value* num_elements = new (Z) Value(call->ArgumentAt(1));
       if (num_elements->BindsToConstant() &&

runtime/vm/compiler/frontend/flow_graph_builder.cc

@@ -3247,6 +3247,87 @@ void EffectGraphVisitor::VisitNativeBodyNode(NativeBodyNode* node) {
         length_load->set_recognized_kind(MethodRecognizer::kObjectArrayLength);
         return ReturnDefinition(length_load);
       }
+      case MethodRecognizer::kListFactory: {
+        // factory List<E>([int length]) {
+        //   return (:arg_desc.positional_count == 2) ? new _List<E>(length)
+        //                                            : new _GrowableList<E>(0);
+        // }
+        ASSERT(owner_->parsed_function().has_arg_desc_var());
+        const auto type_args_parameter = node->scope()->LookupVariable(
+            Symbols::TypeArgumentsParameter(), true);
+        const auto length_parameter =
+            node->scope()->LookupVariable(Symbols::Length(), true);
+        const Library& core_lib = Library::Handle(Z, Library::CoreLibrary());
+
+        // Build: :arg_desc.positional_count == 2
+        TestGraphVisitor comparison(owner(), token_pos);
+        auto arg_descriptor = comparison.Bind(new (Z) LoadLocalInstr(
+            *owner_->parsed_function().arg_desc_var(), token_pos));
+        auto positional_count = comparison.Bind(new (Z) LoadFieldInstr(
+            arg_descriptor, ArgumentsDescriptor::positional_count_offset(),
+            AbstractType::ZoneHandle(Z, Type::SmiType()), token_pos));
+        auto constant_1 = comparison.Bind(
+            new (Z) ConstantInstr(Smi::ZoneHandle(Z, Smi::New(2))));
+        comparison.ReturnDefinition(new (Z) StrictCompareInstr(
+            token_pos, Token::kEQ_STRICT, positional_count, constant_1, false,
+            owner()->GetNextDeoptId()));  // No number check.
+
+        // Build: :expr_temp = new _List<E>(length)
+        ValueGraphVisitor allocate_non_growable(owner());
+        {
+          auto arguments = new (Z) ZoneGrowableArray<PushArgumentInstr*>(Z, 2);
+          arguments->Add(
+              allocate_non_growable.PushArgument(allocate_non_growable.Bind(
+                  new (Z) LoadLocalInstr(*type_args_parameter, token_pos))));
+          arguments->Add(
+              allocate_non_growable.PushArgument(allocate_non_growable.Bind(
+                  new (Z) LoadLocalInstr(*length_parameter, token_pos))));
+          const Class& cls = Class::Handle(
+              Z, core_lib.LookupClass(
+                     Library::PrivateCoreLibName(Symbols::_List())));
+          ASSERT(!cls.IsNull());
+          const intptr_t kTypeArgsLen = 0;
+          const Function& func = Function::ZoneHandle(
+              Z, cls.LookupFactoryAllowPrivate(Symbols::_ListFactory()));
+          ASSERT(!func.IsNull());
+          allocate_non_growable.ReturnDefinition(new (Z) StaticCallInstr(
+              token_pos, func, kTypeArgsLen,
+              Object::null_array(),  // No names.
+              arguments, owner()->ic_data_array(), owner()->GetNextDeoptId(),
+              ICData::kStatic));
+          allocate_non_growable.Do(
+              BuildStoreExprTemp(allocate_non_growable.value(), token_pos));
+        }
+
+        // Build: :expr_temp = new _GrowableList<E>(0)
+        ValueGraphVisitor allocate_growable(owner());
+        {
+          auto arguments = new (Z) ZoneGrowableArray<PushArgumentInstr*>(Z, 1);
+          arguments->Add(allocate_growable.PushArgument(allocate_growable.Bind(
+              new (Z) LoadLocalInstr(*type_args_parameter, token_pos))));
+          arguments->Add(allocate_growable.PushArgument(allocate_growable.Bind(
+              new (Z) ConstantInstr(Smi::ZoneHandle(Z, Smi::New(0))))));
+          const Class& cls = Class::Handle(
+              Z, core_lib.LookupClass(
+                     Library::PrivateCoreLibName(Symbols::_GrowableList())));
+          ASSERT(!cls.IsNull());
+          const intptr_t kTypeArgsLen = 0;
+          const Function& func = Function::ZoneHandle(
+              Z,
+              cls.LookupFactoryAllowPrivate(Symbols::_GrowableListFactory()));
+          ASSERT(!func.IsNull());
+          allocate_growable.ReturnDefinition(new (Z) StaticCallInstr(
+              token_pos, func, kTypeArgsLen,
+              Object::null_array(),  // No names.
+              arguments, owner()->ic_data_array(), owner()->GetNextDeoptId(),
+              ICData::kStatic));
+          allocate_growable.Do(
+              BuildStoreExprTemp(allocate_growable.value(), token_pos));
+        }
+
+        Join(comparison, allocate_non_growable, allocate_growable);
+        return ReturnDefinition(BuildLoadExprTemp(token_pos));
+      }
       case MethodRecognizer::kObjectArrayAllocate: {
         LocalVariable* type_args_parameter = node->scope()->LookupVariable(
             Symbols::TypeArgumentsParameter(), true);

runtime/vm/compiler/frontend/flow_graph_builder.h

@@ -492,6 +492,8 @@ class ValueGraphVisitor : public EffectGraphVisitor {
   Value* value_;
 
  private:
+  friend class EffectGraphVisitor;
+
   // Helper to set the output state to return a Value.
   virtual void ReturnValue(Value* value) { value_ = value; }
 
@@ -539,6 +541,8 @@ class TestGraphVisitor : public ValueGraphVisitor {
   TokenPosition condition_token_pos() const { return condition_token_pos_; }
 
  private:
+  friend class EffectGraphVisitor;
+
   // Construct and concatenate a Branch instruction to this graph fragment.
   // Closes the fragment and sets the output parameters.
   virtual void ReturnValue(Value* value);

runtime/vm/compiler/frontend/kernel_to_il.cc

@@ -2135,6 +2135,71 @@ Fragment FlowGraphBuilder::NativeFunctionBody(intptr_t first_positional_offset,
                               Array::length_offset(),
                               Type::ZoneHandle(Z, Type::SmiType()), kSmiCid);
       break;
+    case MethodRecognizer::kListFactory: {
+      // factory List<E>([int length]) {
+      //   return (:arg_desc.positional_count == 2) ? new _List<E>(length)
+      //                                            : new _GrowableList<E>(0);
+      // }
+      const Library& core_lib = Library::Handle(Z, Library::CoreLibrary());
+
+      TargetEntryInstr *allocate_non_growable, *allocate_growable;
+
+      body += LoadArgDescriptor();
+      body +=
+          LoadField(ArgumentsDescriptor::positional_count_offset(), kSmiCid);
+      body += IntConstant(2);
+      body += BranchIfStrictEqual(&allocate_non_growable, &allocate_growable);
+
+      JoinEntryInstr* join = BuildJoinEntry();
+
+      {
+        const Class& cls = Class::Handle(
+            Z, core_lib.LookupClass(
+                   Library::PrivateCoreLibName(Symbols::_List())));
+        ASSERT(!cls.IsNull());
+        const Function& func = Function::ZoneHandle(
+            Z, cls.LookupFactoryAllowPrivate(Symbols::_ListFactory()));
+        ASSERT(!func.IsNull());
+
+        Fragment allocate(allocate_non_growable);
+        allocate += LoadLocal(scopes_->type_arguments_variable);
+        allocate += PushArgument();
+        allocate += LoadLocal(LookupVariable(first_positional_offset));
+        allocate += PushArgument();
+        allocate +=
+            StaticCall(TokenPosition::kNoSource, func, 2, ICData::kStatic);
+        allocate += StoreLocal(TokenPosition::kNoSource,
+                               parsed_function_->expression_temp_var());
+        allocate += Drop();
+        allocate += Goto(join);
+      }
+
+      {
+        const Class& cls = Class::Handle(
+            Z, core_lib.LookupClass(
+                   Library::PrivateCoreLibName(Symbols::_GrowableList())));
+        ASSERT(!cls.IsNull());
+        const Function& func = Function::ZoneHandle(
+            Z, cls.LookupFactoryAllowPrivate(Symbols::_GrowableListFactory()));
+        ASSERT(!func.IsNull());
+
+        Fragment allocate(allocate_growable);
+        allocate += LoadLocal(scopes_->type_arguments_variable);
+        allocate += PushArgument();
+        allocate += IntConstant(0);
+        allocate += PushArgument();
+        allocate +=
+            StaticCall(TokenPosition::kNoSource, func, 2, ICData::kStatic);
+        allocate += StoreLocal(TokenPosition::kNoSource,
+                               parsed_function_->expression_temp_var());
+        allocate += Drop();
+        allocate += Goto(join);
+      }
+
+      body = Fragment(body.entry, join);
+      body += LoadLocal(parsed_function_->expression_temp_var());
+      break;
+    }
     case MethodRecognizer::kObjectArrayAllocate:
       body += LoadLocal(scopes_->type_arguments_variable);
       body += LoadLocal(LookupVariable(first_positional_offset));

runtime/vm/compiler/method_recognizer.h

@@ -19,6 +19,7 @@ namespace dart {
   V(::, identical, ObjectIdentical, Bool, 0x49c6e96a)                          \
   V(ClassID, getID, ClassIDgetID, Smi, 0x7b18b257)                             \
   V(Object, Object., ObjectConstructor, Dynamic, 0x681617fe)                   \
+  V(List, ., ListFactory, Dynamic, 0x629f8324)                                 \
   V(_List, ., ObjectArrayAllocate, Array, 0x2121902f)                          \
   V(_TypedList, _getInt8, ByteArrayBaseGetInt8, Smi, 0x7041895a)               \
   V(_TypedList, _getUint8, ByteArrayBaseGetUint8, Smi, 0x336fa3ea)             \