Hydrogen array constructor cleanup and improvements

src/code-stubs-hydrogen.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 88 matching lines @@
     IfBuilder checker_;
   };
 
   enum ArgumentClass {
     NONE,
     SINGLE,
     MULTIPLE
   };
 
   HValue* BuildArrayConstructor(ElementsKind kind,
+                                bool ensure_context,

[danno, 2013/06/27 08:53:04]

Turn this boolean  parameter into an enum, it makes it much easier to understand
at the caller site what's going on.

[mvstanton, 2013/06/27 15:33:29]

Done.

                                 bool disable_allocation_sites,

[danno, 2013/06/27 08:53:04]

Turn this boolean  parameter into an enum, it makes it much easier to understand
at the caller site what's going on. In this case, you may want to make it part
of AllocationSite, e.g. AllocationSite::MementoState with values
AllocationSite::MEMENTOS_ENABLED and AllocationSite::MEMENTOS_DISABLED

[mvstanton, 2013/06/27 15:33:29]

Done.

                                 ArgumentClass argument_class);
+  HValue* BuildArrayConstructorImpl(ElementsKind kind,
+                                    HValue* constructor,
+                                    bool disable_allocation_sites,

[danno, 2013/06/27 08:53:04]

Same as above

[mvstanton, 2013/06/27 15:33:29]

Done.

+                                    ArgumentClass argument_class);
   HValue* BuildInternalArrayConstructor(ElementsKind kind,
                                         ArgumentClass argument_class);
 
  private:
   HValue* BuildArraySingleArgumentConstructor(JSArrayBuilder* builder);
   HValue* BuildArrayNArgumentsConstructor(JSArrayBuilder* builder,
                                           ElementsKind kind);
 
   SmartArrayPointer<HParameter*> parameters_;
   HValue* arguments_length_;
@@ skipping 406 matching lines @@
 
   return js_array;
 }
 
 
 Handle<Code> TransitionElementsKindStub::GenerateCode() {
   return DoGenerateCode(this);
 }
 
 HValue* CodeStubGraphBuilderBase::BuildArrayConstructor(
-    ElementsKind kind, bool disable_allocation_sites,
+    ElementsKind kind,
+    bool ensure_context,
+    bool disable_allocation_sites,
     ArgumentClass argument_class) {
   HValue* constructor = GetParameter(ArrayConstructorStubBase::kConstructor);
+  if (ensure_context) {
+    HInstruction* array_function = BuildGetArrayFunction(context());
+    ArrayContextChecker checker(this, constructor, array_function);
+    return BuildArrayConstructorImpl(kind, constructor,

[danno, 2013/06/27 08:53:04]

the return clause is common code, how about factoring it out and putting it
after the if

[mvstanton, 2013/06/27 15:33:29]

Thanks, I initially tried that and ran into some IfBuilder problems, but now it
works...I could eliminate BuildArrayConstructorImpl too.

+                                     disable_allocation_sites, argument_class);
+  } else {
+    return BuildArrayConstructorImpl(kind, constructor,
+                                     disable_allocation_sites, argument_class);
+  }
+}
+
+
+HValue* CodeStubGraphBuilderBase::BuildArrayConstructorImpl(
+    ElementsKind kind,
+    HValue* constructor,
+    bool disable_allocation_sites,
+    ArgumentClass argument_class) {
   HValue* property_cell = GetParameter(ArrayConstructorStubBase::kPropertyCell);
-  HInstruction* array_function = BuildGetArrayFunction(context());
-
-  ArrayContextChecker(this, constructor, array_function);
-  JSArrayBuilder array_builder(this, kind, property_cell,
+  JSArrayBuilder array_builder(this, kind, property_cell, constructor,
                                disable_allocation_sites);
   HValue* result = NULL;
   switch (argument_class) {
     case NONE:
       result = array_builder.AllocateEmptyArray();
       break;
     case SINGLE:
       result = BuildArraySingleArgumentConstructor(&array_builder);
       break;
     case MULTIPLE:
       result = BuildArrayNArgumentsConstructor(&array_builder, kind);
       break;
   }
+

[danno, 2013/06/27 08:53:04]

Not sure this additional line makes a big difference :-)

[mvstanton, 2013/06/27 15:33:29]

Done.

   return result;
 }
 
 
 HValue* CodeStubGraphBuilderBase::BuildInternalArrayConstructor(
     ElementsKind kind, ArgumentClass argument_class) {
   HValue* constructor = GetParameter(
       InternalArrayConstructorStubBase::kConstructor);
   JSArrayBuilder array_builder(this, kind, constructor);
 
@@ skipping 77 matching lines @@
 
   AddInstruction(new(zone()) HStoreKeyed(elements, key, argument, kind));
   builder.EndBody();
   return new_object;
 }
 
 
 template <>
 HValue* CodeStubGraphBuilder<ArrayNoArgumentConstructorStub>::BuildCodeStub() {
   ElementsKind kind = casted_stub()->elements_kind();
+  bool ensure_context = casted_stub()->ensure_context();
   bool disable_allocation_sites = casted_stub()->disable_allocation_sites();
-  return BuildArrayConstructor(kind, disable_allocation_sites, NONE);
+  return BuildArrayConstructor(kind, ensure_context, disable_allocation_sites,
+                               NONE);
 }
 
 
 Handle<Code> ArrayNoArgumentConstructorStub::GenerateCode() {
   return DoGenerateCode(this);
 }
 
 
 template <>
 HValue* CodeStubGraphBuilder<ArraySingleArgumentConstructorStub>::
     BuildCodeStub() {
   ElementsKind kind = casted_stub()->elements_kind();
+  bool ensure_context = casted_stub()->ensure_context();
   bool disable_allocation_sites = casted_stub()->disable_allocation_sites();
-  return BuildArrayConstructor(kind, disable_allocation_sites, SINGLE);
+  return BuildArrayConstructor(kind, ensure_context, disable_allocation_sites,
+                               SINGLE);
 }
 
 
 Handle<Code> ArraySingleArgumentConstructorStub::GenerateCode() {
   return DoGenerateCode(this);
 }
 
 
 template <>
 HValue* CodeStubGraphBuilder<ArrayNArgumentsConstructorStub>::BuildCodeStub() {
   ElementsKind kind = casted_stub()->elements_kind();
+  bool ensure_context = casted_stub()->ensure_context();
   bool disable_allocation_sites = casted_stub()->disable_allocation_sites();
-  return BuildArrayConstructor(kind, disable_allocation_sites, MULTIPLE);
+  return BuildArrayConstructor(kind, ensure_context, disable_allocation_sites,
+                               MULTIPLE);
 }
 
 
 Handle<Code> ArrayNArgumentsConstructorStub::GenerateCode() {
   return DoGenerateCode(this);
 }
 
 
 template <>
 HValue* CodeStubGraphBuilder<InternalArrayNoArgumentConstructorStub>::
@@ skipping 74 matching lines @@
   return graph()->GetConstant0();
 }
 
 
 Handle<Code> ToBooleanStub::GenerateCode() {
   return DoGenerateCode(this);
 }
 
 
 } }  // namespace v8::internal