[CSA] Refactor NonNumberToNumberOrNumeric

We used to have a loop that it was used for JSReceivers. However, this
was not used as a proper loop since at most it would have two loop
iterations. Then, it could be changed to a Branch case.

Since I was refactoring the method, I also de-duplicated code by using
the common code from PlainPrimitiveNonNumberToNumber. In order to do so,
said method was renamed to TryPlain... and was reworked as well.

Bug: v8:6949, v8:10933
Change-Id: I860601a3b9e8bdeed052dcd237a767ac7ed80c92
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2435110
Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70202}

src/codegen/code-stub-assembler.cc

@@ -6755,79 +6755,30 @@ TNode<String> CodeStubAssembler::NumberToString(TNode<Number> input) {
   return result.value();
 }
 
-// TODO(solanes, v8:6949): Refactor this to check for JSReceivers first. If we
-// have a JSReceiver, extract the primitive and fallthrough. Otherwise, continue
-// asking for the other instance types. This will make it so that we can remove
-// the loop (which was looping at most once). Also, see if we can make use of
-// PlainPrimitiveNonNumberToNumber to de-duplicate code, maybe changing it to a
-// TryPlainPrimitiveNonNumberToNumber with a Label* as a parameter.
 TNode<Numeric> CodeStubAssembler::NonNumberToNumberOrNumeric(
     TNode<Context> context, TNode<HeapObject> input, Object::Conversion mode,
     BigIntHandling bigint_handling) {
   CSA_ASSERT(this, Word32BinaryNot(IsHeapNumber(input)));
 
-  // We might need to loop once here due to ToPrimitive conversions.
   TVARIABLE(HeapObject, var_input, input);
   TVARIABLE(Numeric, var_result);
-  Label loop(this, &var_input);
-  Label end(this);
-  Goto(&loop);
-  BIND(&loop);
-  {
-    // Load the current {input} value (known to be a HeapObject).
-    TNode<HeapObject> input = var_input.value();
+  TVARIABLE(Uint16T, instance_type, LoadInstanceType(var_input.value()));
+  Label end(this), if_inputisreceiver(this, Label::kDeferred),
+      if_inputisnotreceiver(this);
 
-    // Dispatch on the {input} instance type.
-    TNode<Uint16T> input_instance_type = LoadInstanceType(input);
-    Label if_inputisstring(this), if_inputisoddball(this),
-        if_inputisbigint(this), if_inputisreceiver(this, Label::kDeferred),
-        if_inputisother(this, Label::kDeferred);
-    GotoIf(IsStringInstanceType(input_instance_type), &if_inputisstring);
-    GotoIf(IsBigIntInstanceType(input_instance_type), &if_inputisbigint);
-    GotoIf(InstanceTypeEqual(input_instance_type, ODDBALL_TYPE),
-           &if_inputisoddball);
-    Branch(IsJSReceiverInstanceType(input_instance_type), &if_inputisreceiver,
-           &if_inputisother);
-
-    BIND(&if_inputisstring);
-    {
-      // The {input} is a String, use the fast stub to convert it to a Number.
-      TNode<String> string_input = CAST(input);
-      var_result = StringToNumber(string_input);
-      Goto(&end);
-    }
-
-    BIND(&if_inputisbigint);
-    if (mode == Object::Conversion::kToNumeric) {
-      var_result = CAST(input);
-      Goto(&end);
-    } else {
-      DCHECK_EQ(mode, Object::Conversion::kToNumber);
-      if (bigint_handling == BigIntHandling::kThrow) {
-        Goto(&if_inputisother);
-      } else {
-        DCHECK_EQ(bigint_handling, BigIntHandling::kConvertToNumber);
-        var_result =
-            CAST(CallRuntime(Runtime::kBigIntToNumber, context, input));
-        Goto(&end);
-      }
-    }
-
-    BIND(&if_inputisoddball);
-    {
-      // The {input} is an Oddball, we just need to load the Number value of it.
-      var_result = LoadObjectField<Number>(input, Oddball::kToNumberOffset);
-      Goto(&end);
-    }
+  // We need to handle JSReceiver first since we might need to do two
+  // conversions due to ToPritmive.
+  Branch(IsJSReceiverInstanceType(instance_type.value()), &if_inputisreceiver,
+         &if_inputisnotreceiver);
 
   BIND(&if_inputisreceiver);
   {
-      // The {input} is a JSReceiver, we need to convert it to a Primitive
-      // first using the ToPrimitive type conversion, preferably yielding a
-      // Number.
+    // The {var_input.value()} is a JSReceiver, we need to convert it to a
+    // Primitive first using the ToPrimitive type conversion, preferably
+    // yielding a Number.
     Callable callable = CodeFactory::NonPrimitiveToPrimitive(
         isolate(), ToPrimitiveHint::kNumber);
-      TNode<Object> result = CallStub(callable, context, input);
+    TNode<Object> result = CallStub(callable, context, var_input.value());
 
     // Check if the {result} is already a Number/Numeric.
     Label if_done(this), if_notdone(this);
@@ -6848,31 +6799,67 @@ TNode<Numeric> CodeStubAssembler::NonNumberToNumberOrNumeric(
       // We now have a Primitive {result}, but it's not yet a
       // Number/Numeric.
       var_input = CAST(result);
-        Goto(&loop);
+      // We have a new input. Redo the check and reload instance_type.
+      CSA_ASSERT(this, Word32BinaryNot(IsHeapNumber(var_input.value())));
+      instance_type = LoadInstanceType(var_input.value());
+      Goto(&if_inputisnotreceiver);
+    }
+  }
+
+  BIND(&if_inputisnotreceiver);
+  {
+    Label not_plain_primitive(this), if_inputisbigint(this),
+        if_inputisother(this, Label::kDeferred);
+
+    // String and Oddball cases.
+    TVARIABLE(Number, var_result_number);
+    TryPlainPrimitiveNonNumberToNumber(var_input.value(), &var_result_number,
+                                       &not_plain_primitive);
+    var_result = var_result_number.value();
+    Goto(&end);
+
+    BIND(&not_plain_primitive);
+    {
+      Branch(IsBigIntInstanceType(instance_type.value()), &if_inputisbigint,
+             &if_inputisother);
+
+      BIND(&if_inputisbigint);
+      {
+        if (mode == Object::Conversion::kToNumeric) {
+          var_result = CAST(var_input.value());
+          Goto(&end);
+        } else {
+          DCHECK_EQ(mode, Object::Conversion::kToNumber);
+          if (bigint_handling == BigIntHandling::kThrow) {
+            Goto(&if_inputisother);
+          } else {
+            DCHECK_EQ(bigint_handling, BigIntHandling::kConvertToNumber);
+            var_result = CAST(CallRuntime(Runtime::kBigIntToNumber, context,
+                                          var_input.value()));
+            Goto(&end);
+          }
         }
       }
 
       BIND(&if_inputisother);
       {
-      // The {input} is something else (e.g. Symbol), let the runtime figure
-      // out the correct exception.
-      // Note: We cannot tail call to the runtime here, as js-to-wasm
-      // trampolines also use this code currently, and they declare all
-      // outgoing parameters as untagged, while we would push a tagged
-      // object here.
+        // The {var_input.value()} is something else (e.g. Symbol), let the
+        // runtime figure out the correct exception. Note: We cannot tail call
+        // to the runtime here, as js-to-wasm trampolines also use this code
+        // currently, and they declare all outgoing parameters as untagged,
+        // while we would push a tagged object here.
         auto function_id = mode == Object::Conversion::kToNumber
                                ? Runtime::kToNumber
                                : Runtime::kToNumeric;
-      var_result = CAST(CallRuntime(function_id, context, input));
+        var_result = CAST(CallRuntime(function_id, context, var_input.value()));
         Goto(&end);
       }
     }
+  }
 
   BIND(&end);
   if (mode == Object::Conversion::kToNumber) {
     CSA_ASSERT(this, IsNumber(var_result.value()));
-  } else {
-    DCHECK_EQ(mode, Object::Conversion::kToNumeric);
   }
   return var_result.value();
 }
@@ -6884,36 +6871,29 @@ TNode<Number> CodeStubAssembler::NonNumberToNumber(
       context, input, Object::Conversion::kToNumber, bigint_handling));
 }
 
-TNode<Number> CodeStubAssembler::PlainPrimitiveNonNumberToNumber(
-    TNode<HeapObject> input) {
+void CodeStubAssembler::TryPlainPrimitiveNonNumberToNumber(
+    TNode<HeapObject> input, TVariable<Number>* var_result, Label* if_bailout) {
   CSA_ASSERT(this, Word32BinaryNot(IsHeapNumber(input)));
-  TVARIABLE(Number, var_result);
   Label done(this);
 
   // Dispatch on the {input} instance type.
   TNode<Uint16T> input_instance_type = LoadInstanceType(input);
-  Label if_inputisstring(this), if_inputisoddball(this);
+  Label if_inputisstring(this);
   GotoIf(IsStringInstanceType(input_instance_type), &if_inputisstring);
-  CSA_ASSERT(this, InstanceTypeEqual(input_instance_type, ODDBALL_TYPE));
-  Goto(&if_inputisoddball);
+  GotoIfNot(InstanceTypeEqual(input_instance_type, ODDBALL_TYPE), if_bailout);
 
-  BIND(&if_inputisstring);
-  {
-    // The {input} is a String, use the fast stub to convert it to a Number.
-    TNode<String> string_input = CAST(input);
-    var_result = StringToNumber(string_input);
+  // The {input} is an Oddball, we just need to load the Number value of it.
+  *var_result = LoadObjectField<Number>(input, Oddball::kToNumberOffset);
   Goto(&done);
-  }
 
-  BIND(&if_inputisoddball);
+  BIND(&if_inputisstring);
   {
-    // The {input} is an Oddball, we just need to load the Number value of it.
-    var_result = LoadObjectField<Number>(input, Oddball::kToNumberOffset);
+    // The {input} is a String, use the fast stub to convert it to a Number.
+    *var_result = StringToNumber(CAST(input));
     Goto(&done);
   }
 
   BIND(&done);
-  return var_result.value();
 }
 
 TNode<Numeric> CodeStubAssembler::NonNumberToNumeric(
@@ -6981,7 +6961,7 @@ TNode<Number> CodeStubAssembler::ToNumber(TNode<Context> context,
 
 TNode<Number> CodeStubAssembler::PlainPrimitiveToNumber(TNode<Object> input) {
   TVARIABLE(Number, var_result);
-  Label end(this);
+  Label end(this), fallback(this);
 
   Label not_smi(this, Label::kDeferred);
   GotoIfNot(TaggedIsSmi(input), &not_smi);
@@ -7001,8 +6981,10 @@ TNode<Number> CodeStubAssembler::PlainPrimitiveToNumber(TNode<Object> input) {
 
     BIND(&not_heap_number);
     {
-      var_result = PlainPrimitiveNonNumberToNumber(input_ho);
+      TryPlainPrimitiveNonNumberToNumber(input_ho, &var_result, &fallback);
       Goto(&end);
+      BIND(&fallback);
+      Unreachable();
     }
   }
 

src/codegen/code-stub-assembler.h

@@ -3672,8 +3672,12 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
       TNode<Object> value, WriteBarrierMode barrier_mode = UPDATE_WRITE_BARRIER,
       int additional_offset = 0);
 
-  // Converts {input} to a number. {input} must be a plain primitve.
-  TNode<Number> PlainPrimitiveNonNumberToNumber(TNode<HeapObject> input);
+  // Converts {input} to a number if {input} is a plain primitve (i.e. String or
+  // Oddball) and stores the result in {var_result}. Otherwise, it bails out to
+  // {if_bailout}.
+  void TryPlainPrimitiveNonNumberToNumber(TNode<HeapObject> input,
+                                          TVariable<Number>* var_result,
+                                          Label* if_bailout);
 };
 
 class V8_EXPORT_PRIVATE CodeStubArguments {