[csa][cleanup] Remove ParameterMode from ExtractFixedArray

Drive-by:
 * Updated nullptr to using base::Optional.
 * Remove ParameterMode use in CloneFixedArray.

Bug: v8:9708, v8:6949
Change-Id: I0a98ded0a5d25df078cccbba1385d177652d1cf4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2324242Reviewed-by: Dan Elphick <delphick@chromium.org>
Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69120}

src/builtins/growable-fixed-array-gen.cc

@@ -91,7 +91,9 @@ TNode<FixedArray> GrowableFixedArray::ResizeFixedArray(
   CodeStubAssembler::ExtractFixedArrayFlags flags;
   flags |= CodeStubAssembler::ExtractFixedArrayFlag::kFixedArrays;
   TNode<FixedArray> to_array = CAST(ExtractFixedArray(
-      from_array, nullptr, element_count, new_capacity, flags));
+      from_array, base::Optional<TNode<IntPtrT>>(base::nullopt),
+      base::Optional<TNode<IntPtrT>>(element_count),
+      base::Optional<TNode<IntPtrT>>(new_capacity), flags));
 
   return to_array;
 }

src/codegen/code-stub-assembler.cc

@@ -3698,7 +3698,9 @@ TNode<JSArray> CodeStubAssembler::ExtractFastJSArray(TNode<Context> context,
   TNode<Map> array_map = LoadJSArrayElementsMap(elements_kind, native_context);
 
   TNode<FixedArrayBase> new_elements = ExtractFixedArray(
-      LoadElements(array), begin, count, base::nullopt,
+      LoadElements(array), base::Optional<TNode<BInt>>(begin),
+      base::Optional<TNode<BInt>>(count),
+      base::Optional<TNode<BInt>>(base::nullopt),
       ExtractFixedArrayFlag::kAllFixedArrays, nullptr, elements_kind);
 
   TNode<JSArray> result = AllocateJSArray(
@@ -3714,8 +3716,6 @@ TNode<JSArray> CodeStubAssembler::CloneFastJSArray(
   // function is also used to copy boilerplates even when the no-elements
   // protector is invalid. This function should be renamed to reflect its uses.
 
-  // TODO(v8:9708): remove ParameterMode
-  ParameterMode mode = OptimalParameterMode();
   TNode<Number> length = LoadJSArrayLength(array);
   TNode<FixedArrayBase> new_elements;
   TVARIABLE(FixedArrayBase, var_new_elements);
@@ -3733,11 +3733,13 @@ TNode<JSArray> CodeStubAssembler::CloneFastJSArray(
   }
 
   // Simple extraction that preserves holes.
-  new_elements =
-      ExtractFixedArray(LoadElements(array), IntPtrOrSmiConstant(0, mode),
-                        TaggedToParameter<BInt>(CAST(length)), nullptr,
-                        ExtractFixedArrayFlag::kAllFixedArraysDontCopyCOW, mode,
-                        nullptr, var_elements_kind.value());
+  new_elements = ExtractFixedArray(
+      LoadElements(array),
+      base::Optional<TNode<BInt>>(IntPtrOrSmiConstant<BInt>(0)),
+      base::Optional<TNode<BInt>>(TaggedToParameter<BInt>(CAST(length))),
+      base::Optional<TNode<BInt>>(base::nullopt),
+      ExtractFixedArrayFlag::kAllFixedArraysDontCopyCOW, nullptr,
+      var_elements_kind.value());
   var_new_elements = new_elements;
   Goto(&allocate_jsarray);
 
@@ -3752,9 +3754,11 @@ TNode<JSArray> CodeStubAssembler::CloneFastJSArray(
     // PACKED_ELEMENTS. Also, if we want to replace holes, we must not use
     // ExtractFixedArrayFlag::kDontCopyCOW.
     new_elements = ExtractFixedArray(
-        LoadElements(array), IntPtrOrSmiConstant(0, mode),
-        TaggedToParameter<BInt>(CAST(length)), nullptr,
-        ExtractFixedArrayFlag::kAllFixedArrays, mode, &var_holes_converted);
+        LoadElements(array),
+        base::Optional<TNode<BInt>>(IntPtrOrSmiConstant<BInt>(0)),
+        base::Optional<TNode<BInt>>(TaggedToParameter<BInt>(CAST(length))),
+        base::Optional<TNode<BInt>>(base::nullopt),
+        ExtractFixedArrayFlag::kAllFixedArrays, &var_holes_converted);
     var_new_elements = new_elements;
     // If the array type didn't change, use the original elements kind.
     GotoIfNot(var_holes_converted.value(), &allocate_jsarray);
@@ -4103,15 +4107,19 @@ TNode<FixedArrayBase> CodeStubAssembler::ExtractFixedDoubleArrayFillingHoles(
   return var_result.value();
 }
 
+template <typename TIndex>
 TNode<FixedArrayBase> CodeStubAssembler::ExtractFixedArray(
-    TNode<FixedArrayBase> source, Node* first, Node* count, Node* capacity,
-    ExtractFixedArrayFlags extract_flags, ParameterMode parameter_mode,
-    TVariable<BoolT>* var_holes_converted,
-    base::Optional<TNode<Int32T>> source_runtime_kind) {
+    TNode<FixedArrayBase> source, base::Optional<TNode<TIndex>> first,
+    base::Optional<TNode<TIndex>> count, base::Optional<TNode<TIndex>> capacity,
+    ExtractFixedArrayFlags extract_flags, TVariable<BoolT>* var_holes_converted,
+    base::Optional<TNode<Int32T>> source_elements_kind) {
+  static_assert(
+      std::is_same<TIndex, Smi>::value || std::is_same<TIndex, IntPtrT>::value,
+      "Only Smi or IntPtrT first, count, and capacity are allowed");
   DCHECK(extract_flags & ExtractFixedArrayFlag::kFixedArrays ||
          extract_flags & ExtractFixedArrayFlag::kFixedDoubleArrays);
-  // If we want to replace holes, ExtractFixedArrayFlag::kDontCopyCOW should not
-  // be used, because that disables the iteration which detects holes.
+  // If we want to replace holes, ExtractFixedArrayFlag::kDontCopyCOW should
+  // not be used, because that disables the iteration which detects holes.
   DCHECK_IMPLIES(var_holes_converted != nullptr,
                  !(extract_flags & ExtractFixedArrayFlag::kDontCopyCOW));
   HoleConversionMode convert_holes =
@@ -4122,31 +4130,26 @@ TNode<FixedArrayBase> CodeStubAssembler::ExtractFixedArray(
       (extract_flags & ExtractFixedArrayFlag::kNewSpaceAllocationOnly)
           ? CodeStubAssembler::kNone
           : CodeStubAssembler::kAllowLargeObjectAllocation;
-  if (first == nullptr) {
-    first = IntPtrOrSmiConstant(0, parameter_mode);
+  if (!first) {
+    first = IntPtrOrSmiConstant<TIndex>(0);
   }
-  if (count == nullptr) {
+  if (!count) {
     count = IntPtrOrSmiSub(
-        TaggedToParameter(LoadFixedArrayBaseLength(source), parameter_mode),
-        first, parameter_mode);
+        TaggedToParameter<TIndex>(LoadFixedArrayBaseLength(source)), *first);
 
-    CSA_ASSERT(
-        this, IntPtrOrSmiLessThanOrEqual(IntPtrOrSmiConstant(0, parameter_mode),
-                                         count, parameter_mode));
+    CSA_ASSERT(this, IntPtrOrSmiLessThanOrEqual(IntPtrOrSmiConstant<TIndex>(0),
+                                                *count));
   }
-  if (capacity == nullptr) {
-    capacity = count;
+  if (!capacity) {
+    capacity = *count;
   } else {
     CSA_ASSERT(this, Word32BinaryNot(IntPtrOrSmiGreaterThan(
-                         IntPtrOrSmiAdd(first, count, parameter_mode), capacity,
-                         parameter_mode)));
+                         IntPtrOrSmiAdd(*first, *count), *capacity)));
   }
 
   Label if_fixed_double_array(this), empty(this), done(this, &var_result);
   TNode<Map> source_map = LoadMap(source);
-  GotoIf(IntPtrOrSmiEqual(IntPtrOrSmiConstant(0, parameter_mode), capacity,
-                          parameter_mode),
-         &empty);
+  GotoIf(IntPtrOrSmiEqual(IntPtrOrSmiConstant<TIndex>(0), *capacity), &empty);
 
   if (extract_flags & ExtractFixedArrayFlag::kFixedDoubleArrays) {
     if (extract_flags & ExtractFixedArrayFlag::kFixedArrays) {
@@ -4156,13 +4159,15 @@ TNode<FixedArrayBase> CodeStubAssembler::ExtractFixedArray(
     }
   }
 
+  const ParameterMode parameter_mode =
+      std::is_same<TIndex, Smi>::value ? SMI_PARAMETERS : INTPTR_PARAMETERS;
   if (extract_flags & ExtractFixedArrayFlag::kFixedArrays) {
     // Here we can only get |source| as FixedArray, never FixedDoubleArray.
     // PACKED_ELEMENTS is used to signify that the source is a FixedArray.
     TNode<FixedArray> to_elements = ExtractToFixedArray(
-        source, first, count, capacity, source_map, PACKED_ELEMENTS,
+        source, *first, *count, *capacity, source_map, PACKED_ELEMENTS,
         allocation_flags, extract_flags, parameter_mode, convert_holes,
-        var_holes_converted, source_runtime_kind);
+        var_holes_converted, source_elements_kind);
     var_result = to_elements;
     Goto(&done);
   }
@@ -4173,7 +4178,7 @@ TNode<FixedArrayBase> CodeStubAssembler::ExtractFixedArray(
 
     if (convert_holes == HoleConversionMode::kConvertToUndefined) {
       TNode<FixedArrayBase> to_elements = ExtractFixedDoubleArrayFillingHoles(
-          source, first, count, capacity, source_map, var_holes_converted,
+          source, *first, *count, *capacity, source_map, var_holes_converted,
           allocation_flags, extract_flags, parameter_mode);
       var_result = to_elements;
     } else {
@@ -4182,12 +4187,12 @@ TNode<FixedArrayBase> CodeStubAssembler::ExtractFixedArray(
       // matter.
       ElementsKind kind = PACKED_DOUBLE_ELEMENTS;
       TNode<FixedArrayBase> to_elements = AllocateFixedArray(
-          kind, capacity, parameter_mode, allocation_flags, source_map);
-      FillFixedArrayWithValue(kind, to_elements, count, capacity,
+          kind, *capacity, parameter_mode, allocation_flags, source_map);
+      FillFixedArrayWithValue(kind, to_elements, *count, *capacity,
                               RootIndex::kTheHoleValue, parameter_mode);
       CopyElements(kind, to_elements, IntPtrConstant(0), source,
-                   ParameterToIntPtr(first, parameter_mode),
-                   ParameterToIntPtr(count, parameter_mode));
+                   ParameterToIntPtr(*first, parameter_mode),
+                   ParameterToIntPtr(*count, parameter_mode));
       var_result = to_elements;
     }
 
@@ -4206,6 +4211,18 @@ TNode<FixedArrayBase> CodeStubAssembler::ExtractFixedArray(
   return var_result.value();
 }
 
+template V8_EXPORT_PRIVATE TNode<FixedArrayBase>
+CodeStubAssembler::ExtractFixedArray<Smi>(
+    TNode<FixedArrayBase>, base::Optional<TNode<Smi>>,
+    base::Optional<TNode<Smi>>, base::Optional<TNode<Smi>>,
+    ExtractFixedArrayFlags, TVariable<BoolT>*, base::Optional<TNode<Int32T>>);
+
+template V8_EXPORT_PRIVATE TNode<FixedArrayBase>
+CodeStubAssembler::ExtractFixedArray<IntPtrT>(
+    TNode<FixedArrayBase>, base::Optional<TNode<IntPtrT>>,
+    base::Optional<TNode<IntPtrT>>, base::Optional<TNode<IntPtrT>>,
+    ExtractFixedArrayFlags, TVariable<BoolT>*, base::Optional<TNode<Int32T>>);
+
 void CodeStubAssembler::InitializePropertyArrayLength(
     TNode<PropertyArray> property_array, TNode<IntPtrT> length) {
   CSA_ASSERT(this, IntPtrGreaterThan(length, IntPtrConstant(0)));
@@ -4769,6 +4786,14 @@ void CodeStubAssembler::CopyPropertyArrayValues(TNode<HeapObject> from_array,
   Comment("] CopyPropertyArrayValues");
 }
 
+TNode<FixedArrayBase> CodeStubAssembler::CloneFixedArray(
+    TNode<FixedArrayBase> source, ExtractFixedArrayFlags flags) {
+  return ExtractFixedArray(
+      source, base::Optional<TNode<BInt>>(IntPtrOrSmiConstant<BInt>(0)),
+      base::Optional<TNode<BInt>>(base::nullopt),
+      base::Optional<TNode<BInt>>(base::nullopt), flags);
+}
+
 Node* CodeStubAssembler::LoadElementAndPrepareForStore(
     TNode<FixedArrayBase> array, TNode<IntPtrT> offset, ElementsKind from_kind,
     ElementsKind to_kind, Label* if_hole) {

src/codegen/code-stub-assembler.h

@@ -2126,45 +2126,16 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
   // * If |source_elements_kind| is given, the function will try to use the
   // runtime elements kind of source to make copy faster. More specifically, it
   // can skip write barriers.
+  template <typename TIndex>
   TNode<FixedArrayBase> ExtractFixedArray(
-      TNode<FixedArrayBase> source, Node* first, Node* count = nullptr,
-      Node* capacity = nullptr,
+      TNode<FixedArrayBase> source, base::Optional<TNode<TIndex>> first,
+      base::Optional<TNode<TIndex>> count = base::nullopt,
+      base::Optional<TNode<TIndex>> capacity = base::nullopt,
       ExtractFixedArrayFlags extract_flags =
           ExtractFixedArrayFlag::kAllFixedArrays,
-      ParameterMode parameter_mode = INTPTR_PARAMETERS,
       TVariable<BoolT>* var_holes_converted = nullptr,
       base::Optional<TNode<Int32T>> source_elements_kind = base::nullopt);
 
-  TNode<FixedArrayBase> ExtractFixedArray(
-      TNode<FixedArrayBase> source, TNode<Smi> first, TNode<Smi> count,
-      base::Optional<TNode<Smi>> capacity,
-      ExtractFixedArrayFlags extract_flags =
-          ExtractFixedArrayFlag::kAllFixedArrays,
-      TVariable<BoolT>* var_holes_converted = nullptr,
-      base::Optional<TNode<Int32T>> source_elements_kind = base::nullopt) {
-    // TODO(solanes): just use capacity when ExtractFixedArray is fully
-    // converted.
-    Node* capacity_node = capacity ? static_cast<Node*>(*capacity) : nullptr;
-    return ExtractFixedArray(source, first, count, capacity_node, extract_flags,
-                             SMI_PARAMETERS, var_holes_converted,
-                             source_elements_kind);
-  }
-
-  TNode<FixedArrayBase> ExtractFixedArray(
-      TNode<FixedArrayBase> source, TNode<IntPtrT> first, TNode<IntPtrT> count,
-      base::Optional<TNode<IntPtrT>> capacity,
-      ExtractFixedArrayFlags extract_flags =
-          ExtractFixedArrayFlag::kAllFixedArrays,
-      TVariable<BoolT>* var_holes_converted = nullptr,
-      base::Optional<TNode<Int32T>> source_elements_kind = base::nullopt) {
-    // TODO(solanes): just use capacity when ExtractFixedArray is fully
-    // converted.
-    Node* capacity_node = capacity ? static_cast<Node*>(*capacity) : nullptr;
-    return ExtractFixedArray(source, first, count, capacity_node, extract_flags,
-                             INTPTR_PARAMETERS, var_holes_converted,
-                             source_elements_kind);
-  }
-
   // Copy a portion of an existing FixedArray or FixedDoubleArray into a new
   // FixedArray, including special appropriate handling for COW arrays.
   // * |source| is either a FixedArray or FixedDoubleArray from which to copy
@@ -2244,11 +2215,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
   TNode<FixedArrayBase> CloneFixedArray(
       TNode<FixedArrayBase> source,
       ExtractFixedArrayFlags flags =
-          ExtractFixedArrayFlag::kAllFixedArraysDontCopyCOW) {
-    ParameterMode mode = OptimalParameterMode();
-    return ExtractFixedArray(source, IntPtrOrSmiConstant(0, mode), nullptr,
-                             nullptr, flags, mode);
-  }
+          ExtractFixedArrayFlag::kAllFixedArraysDontCopyCOW);
 
   // Loads an element from |array| of |from_kind| elements by given |offset|
   // (NOTE: not index!), does a hole check if |if_hole| is provided and

test/cctest/test-code-stub-assembler.cc

@@ -3231,9 +3231,11 @@ TEST(ExtractFixedArrayCOWForceCopy) {
     CodeStubAssembler m(asm_tester.state());
     CodeStubAssembler::ExtractFixedArrayFlags flags;
     flags |= CodeStubAssembler::ExtractFixedArrayFlag::kAllFixedArrays;
-    m.Return(m.ExtractFixedArray(m.CAST(m.Parameter(1)), m.SmiConstant(0),
-                                 nullptr, nullptr, flags,
-                                 CodeStubAssembler::SMI_PARAMETERS));
+    base::Optional<TNode<Smi>> constant(m.SmiConstant(0));
+    m.Return(m.ExtractFixedArray(m.CAST(m.Parameter(1)), constant,
+                                 base::Optional<TNode<Smi>>(base::nullopt),
+                                 base::Optional<TNode<Smi>>(base::nullopt),
+                                 flags));
   }
   FunctionTester ft(asm_tester.GenerateCode(), kNumParams);
 
@@ -3260,9 +3262,11 @@ TEST(ExtractFixedArraySimple) {
     CodeStubAssembler::ExtractFixedArrayFlags flags;
     flags |= CodeStubAssembler::ExtractFixedArrayFlag::kAllFixedArrays;
     flags |= CodeStubAssembler::ExtractFixedArrayFlag::kDontCopyCOW;
-    m.Return(m.ExtractFixedArray(m.CAST(m.Parameter(1)), m.Parameter(2),
-                                 m.Parameter(3), nullptr, flags,
-                                 CodeStubAssembler::SMI_PARAMETERS));
+    base::Optional<TNode<IntPtrT>> p1_untagged(m.SmiUntag(m.Parameter(2)));
+    base::Optional<TNode<IntPtrT>> p2_untagged(m.SmiUntag(m.Parameter(3)));
+    m.Return(m.ExtractFixedArray(
+        m.CAST(m.Parameter(1)), p1_untagged, p2_untagged,
+        base::Optional<TNode<IntPtrT>>(base::nullopt), flags));
   }
   FunctionTester ft(asm_tester.GenerateCode(), kNumParams);
 
@@ -3287,9 +3291,11 @@ TEST(ExtractFixedArraySimpleSmiConstant) {
     CodeStubAssembler::ExtractFixedArrayFlags flags;
     flags |= CodeStubAssembler::ExtractFixedArrayFlag::kAllFixedArrays;
     flags |= CodeStubAssembler::ExtractFixedArrayFlag::kDontCopyCOW;
-    m.Return(m.ExtractFixedArray(m.CAST(m.Parameter(1)), m.SmiConstant(1),
-                                 m.SmiConstant(2), nullptr, flags,
-                                 CodeStubAssembler::SMI_PARAMETERS));
+    base::Optional<TNode<Smi>> constant_1(m.SmiConstant(1));
+    base::Optional<TNode<Smi>> constant_2(m.SmiConstant(2));
+    m.Return(m.ExtractFixedArray(m.CAST(m.Parameter(1)), constant_1, constant_2,
+                                 base::Optional<TNode<Smi>>(base::nullopt),
+                                 flags));
   }
   FunctionTester ft(asm_tester.GenerateCode(), kNumParams);
 
@@ -3311,9 +3317,11 @@ TEST(ExtractFixedArraySimpleIntPtrConstant) {
     CodeStubAssembler::ExtractFixedArrayFlags flags;
     flags |= CodeStubAssembler::ExtractFixedArrayFlag::kAllFixedArrays;
     flags |= CodeStubAssembler::ExtractFixedArrayFlag::kDontCopyCOW;
-    m.Return(m.ExtractFixedArray(m.CAST(m.Parameter(1)), m.IntPtrConstant(1),
-                                 m.IntPtrConstant(2), nullptr, flags,
-                                 CodeStubAssembler::INTPTR_PARAMETERS));
+    base::Optional<TNode<IntPtrT>> constant_1(m.IntPtrConstant(1));
+    base::Optional<TNode<IntPtrT>> constant_2(m.IntPtrConstant(2));
+    m.Return(m.ExtractFixedArray(m.CAST(m.Parameter(1)), constant_1, constant_2,
+                                 base::Optional<TNode<IntPtrT>>(base::nullopt),
+                                 flags));
   }
   FunctionTester ft(asm_tester.GenerateCode(), kNumParams);
 
@@ -3332,10 +3340,12 @@ TEST(ExtractFixedArraySimpleIntPtrConstantNoDoubles) {
   CodeAssemblerTester asm_tester(isolate, kNumParams + 1);  // Include receiver.
   {
     CodeStubAssembler m(asm_tester.state());
+    base::Optional<TNode<IntPtrT>> constant_1(m.IntPtrConstant(1));
+    base::Optional<TNode<IntPtrT>> constant_2(m.IntPtrConstant(2));
     m.Return(m.ExtractFixedArray(
-        m.CAST(m.Parameter(1)), m.IntPtrConstant(1), m.IntPtrConstant(2),
-        nullptr, CodeStubAssembler::ExtractFixedArrayFlag::kFixedArrays,
-        CodeStubAssembler::INTPTR_PARAMETERS));
+        m.CAST(m.Parameter(1)), constant_1, constant_2,
+        base::Optional<TNode<IntPtrT>>(base::nullopt),
+        CodeStubAssembler::ExtractFixedArrayFlag::kFixedArrays));
   }
   FunctionTester ft(asm_tester.GenerateCode(), kNumParams);
 
@@ -3354,8 +3364,8 @@ TEST(ExtractFixedArraySimpleIntPtrParameters) {
   CodeAssemblerTester asm_tester(isolate, kNumParams + 1);  // Include receiver.
   {
     CodeStubAssembler m(asm_tester.state());
-    TNode<IntPtrT> p1_untagged = m.SmiUntag(m.Parameter(2));
-    TNode<IntPtrT> p2_untagged = m.SmiUntag(m.Parameter(3));
+    base::Optional<TNode<IntPtrT>> p1_untagged(m.SmiUntag(m.Parameter(2)));
+    base::Optional<TNode<IntPtrT>> p2_untagged(m.SmiUntag(m.Parameter(3)));
     m.Return(
         m.ExtractFixedArray(m.CAST(m.Parameter(1)), p1_untagged, p2_untagged));
   }