[cleanup] Tnodify number conversion and *Feedback methods

This Tnodifies the CombineFeedback and OverwriteFeedback methods and
changes the TaggedToWord32OrBigInt* ann TaggedToNumeric methods to take
TVariables.

Additionally it refactors bitwise binary operators in
intepreter-generator.cc and builtins-number-gen.cc and puts the common
code in NumberBuiltinsAssembler.

Bug: v8:10021
Change-Id: I3b15ecfadb42b50ffbfd0bd1114197e0fef42e99
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1995387
Commit-Queue: Dan Elphick <delphick@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65762}

src/builtins/builtins-number-gen.cc

@@ -25,29 +25,8 @@ class NumberBuiltinsAssembler : public CodeStubAssembler {
     TNode<Object> right = CAST(Parameter(Descriptor::kRight));
     TNode<Context> context = CAST(Parameter(Descriptor::kContext));
 
-    TVARIABLE(Word32T, var_left_word32);
-    TVARIABLE(Word32T, var_right_word32);
-    TVARIABLE(Object, var_left_maybe_bigint, left);
-    TVARIABLE(Object, var_right_maybe_bigint);
-    Label if_left_number(this), do_number_op(this);
-    Label if_left_bigint(this), do_bigint_op(this);
-
-    TaggedToWord32OrBigInt(context, left, &if_left_number, &var_left_word32,
-                           &if_left_bigint, &var_left_maybe_bigint);
-    BIND(&if_left_number);
-    TaggedToWord32OrBigInt(context, right, &do_number_op, &var_right_word32,
-                           &do_bigint_op, &var_right_maybe_bigint);
-    BIND(&do_number_op);
-    Return(BitwiseOp(var_left_word32.value(), var_right_word32.value(), op));
-
-    // BigInt cases.
-    BIND(&if_left_bigint);
-    TaggedToNumeric(context, right, &do_bigint_op, &var_right_maybe_bigint);
-
-    BIND(&do_bigint_op);
-    Return(CallRuntime(Runtime::kBigIntBinaryOp, context,
-                       var_left_maybe_bigint.value(),
-                       var_right_maybe_bigint.value(), SmiConstant(op)));
+    BinaryOpAssembler binop_asm(state());
+    Return(binop_asm.Generate_BitwiseBinaryOp(op, left, right, context));
   }
 
   template <typename Descriptor>

src/codegen/code-stub-assembler.cc

@@ -5056,7 +5056,7 @@ TNode<Word32T> CodeStubAssembler::TruncateTaggedToWord32(
 void CodeStubAssembler::TaggedToWord32OrBigInt(
     TNode<Context> context, TNode<Object> value, Label* if_number,
     TVariable<Word32T>* var_word32, Label* if_bigint,
-    TVariable<Object>* var_maybe_bigint) {
+    TVariable<BigInt>* var_maybe_bigint) {
   TaggedToWord32OrBigIntImpl<Object::Conversion::kToNumeric>(
       context, value, if_number, var_word32, if_bigint, var_maybe_bigint);
 }
@@ -5067,7 +5067,7 @@ void CodeStubAssembler::TaggedToWord32OrBigInt(
 void CodeStubAssembler::TaggedToWord32OrBigIntWithFeedback(
     TNode<Context> context, TNode<Object> value, Label* if_number,
     TVariable<Word32T>* var_word32, Label* if_bigint,
-    TVariable<Object>* var_maybe_bigint, TVariable<Smi>* var_feedback) {
+    TVariable<BigInt>* var_maybe_bigint, TVariable<Smi>* var_feedback) {
   TaggedToWord32OrBigIntImpl<Object::Conversion::kToNumeric>(
       context, value, if_number, var_word32, if_bigint, var_maybe_bigint,
       var_feedback);
@@ -5077,7 +5077,7 @@ template <Object::Conversion conversion>
 void CodeStubAssembler::TaggedToWord32OrBigIntImpl(
     TNode<Context> context, TNode<Object> value, Label* if_number,
     TVariable<Word32T>* var_word32, Label* if_bigint,
-    TVariable<Object>* var_maybe_bigint, TVariable<Smi>* var_feedback) {
+    TVariable<BigInt>* var_maybe_bigint, TVariable<Smi>* var_feedback) {
   // We might need to loop after conversion.
   TVARIABLE(Object, var_value, value);
   OverwriteFeedback(var_feedback, BinaryOperationFeedback::kNone);
@@ -5139,7 +5139,7 @@ void CodeStubAssembler::TaggedToWord32OrBigIntImpl(
 
     if (conversion == Object::Conversion::kToNumeric) {
       BIND(&is_bigint);
-      *var_maybe_bigint = value;
+      *var_maybe_bigint = CAST(value);
       CombineFeedback(var_feedback, BinaryOperationFeedback::kBigInt);
       Goto(if_bigint);
     }
@@ -6982,26 +6982,24 @@ TNode<BigInt> CodeStubAssembler::ToBigInt(TNode<Context> context,
 }
 
 void CodeStubAssembler::TaggedToNumeric(TNode<Context> context,
-                                        TNode<Object> value, Label* done,
-                                        Variable* var_numeric) {
-  TaggedToNumeric(context, value, done, var_numeric, nullptr);
+                                        TNode<Object> value,
+                                        TVariable<Numeric>* var_numeric) {
+  TaggedToNumeric(context, value, var_numeric, nullptr);
 }
 
-void CodeStubAssembler::TaggedToNumericWithFeedback(TNode<Context> context,
-                                                    TNode<Object> value,
-                                                    Label* done,
-                                                    Variable* var_numeric,
-                                                    Variable* var_feedback) {
+void CodeStubAssembler::TaggedToNumericWithFeedback(
+    TNode<Context> context, TNode<Object> value,
+    TVariable<Numeric>* var_numeric, TVariable<Smi>* var_feedback) {
   DCHECK_NOT_NULL(var_feedback);
-  TaggedToNumeric(context, value, done, var_numeric, var_feedback);
+  TaggedToNumeric(context, value, var_numeric, var_feedback);
 }
 
 void CodeStubAssembler::TaggedToNumeric(TNode<Context> context,
-                                        TNode<Object> value, Label* done,
-                                        Variable* var_numeric,
-                                        Variable* var_feedback) {
-  var_numeric->Bind(value);
-  Label if_smi(this), if_heapnumber(this), if_bigint(this), if_oddball(this);
+                                        TNode<Object> value,
+                                        TVariable<Numeric>* var_numeric,
+                                        TVariable<Smi>* var_feedback) {
+  Label done(this), if_smi(this), if_heapnumber(this), if_bigint(this),
+      if_oddball(this);
   GotoIf(TaggedIsSmi(value), &if_smi);
   TNode<HeapObject> heap_object_value = CAST(value);
   TNode<Map> map = LoadMap(heap_object_value);
@@ -7011,28 +7009,33 @@ void CodeStubAssembler::TaggedToNumeric(TNode<Context> context,
 
   // {heap_object_value} is not a Numeric yet.
   GotoIf(Word32Equal(instance_type, Int32Constant(ODDBALL_TYPE)), &if_oddball);
-  var_numeric->Bind(
+  *var_numeric = CAST(
       CallBuiltin(Builtins::kNonNumberToNumeric, context, heap_object_value));
   OverwriteFeedback(var_feedback, BinaryOperationFeedback::kAny);
-  Goto(done);
+  Goto(&done);
 
   BIND(&if_smi);
+  *var_numeric = CAST(value);
   OverwriteFeedback(var_feedback, BinaryOperationFeedback::kSignedSmall);
-  Goto(done);
+  Goto(&done);
 
   BIND(&if_heapnumber);
+  *var_numeric = CAST(value);
   OverwriteFeedback(var_feedback, BinaryOperationFeedback::kNumber);
-  Goto(done);
+  Goto(&done);
 
   BIND(&if_bigint);
+  *var_numeric = CAST(value);
   OverwriteFeedback(var_feedback, BinaryOperationFeedback::kBigInt);
-  Goto(done);
+  Goto(&done);
 
   BIND(&if_oddball);
   OverwriteFeedback(var_feedback, BinaryOperationFeedback::kNumberOrOddball);
-  var_numeric->Bind(
-      LoadObjectField(heap_object_value, Oddball::kToNumberOffset));
-  Goto(done);
+  *var_numeric =
+      CAST(LoadObjectField(heap_object_value, Oddball::kToNumberOffset));
+  Goto(&done);
+
+  Bind(&done);
 }
 
 // ES#sec-touint32
@@ -9416,24 +9419,22 @@ void CodeStubAssembler::ReportFeedbackUpdate(
 #endif  // V8_TRACE_FEEDBACK_UPDATES
 }
 
-void CodeStubAssembler::OverwriteFeedback(Variable* existing_feedback,
+void CodeStubAssembler::OverwriteFeedback(TVariable<Smi>* existing_feedback,
                                           int new_feedback) {
   if (existing_feedback == nullptr) return;
-  existing_feedback->Bind(SmiConstant(new_feedback));
+  *existing_feedback = SmiConstant(new_feedback);
 }
 
-void CodeStubAssembler::CombineFeedback(Variable* existing_feedback,
+void CodeStubAssembler::CombineFeedback(TVariable<Smi>* existing_feedback,
                                         int feedback) {
   if (existing_feedback == nullptr) return;
-  existing_feedback->Bind(
-      SmiOr(CAST(existing_feedback->value()), SmiConstant(feedback)));
+  *existing_feedback = SmiOr(existing_feedback->value(), SmiConstant(feedback));
 }
 
-void CodeStubAssembler::CombineFeedback(Variable* existing_feedback,
-                                        Node* feedback) {
+void CodeStubAssembler::CombineFeedback(TVariable<Smi>* existing_feedback,
+                                        TNode<Smi> feedback) {
   if (existing_feedback == nullptr) return;
-  existing_feedback->Bind(
-      SmiOr(CAST(existing_feedback->value()), CAST(feedback)));
+  *existing_feedback = SmiOr(existing_feedback->value(), feedback);
 }
 
 void CodeStubAssembler::CheckForAssociatedProtector(SloppyTNode<Name> name,
@@ -10906,7 +10907,7 @@ TNode<Smi> CodeStubAssembler::CollectFeedbackForString(
 
 void CodeStubAssembler::GenerateEqual_Same(SloppyTNode<Object> value,
                                            Label* if_equal, Label* if_notequal,
-                                           Variable* var_type_feedback) {
+                                           TVariable<Smi>* var_type_feedback) {
   // In case of abstract or strict equality checks, we need additional checks
   // for NaN values because they are not considered equal, even if both the
   // left and the right hand side reference exactly the same value.

src/codegen/code-stub-assembler.h

@@ -2356,12 +2356,12 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
   void TaggedToWord32OrBigInt(TNode<Context> context, TNode<Object> value,
                               Label* if_number, TVariable<Word32T>* var_word32,
                               Label* if_bigint,
-                              TVariable<Object>* var_maybe_bigint);
+                              TVariable<BigInt>* var_maybe_bigint);
   void TaggedToWord32OrBigIntWithFeedback(TNode<Context> context,
                                           TNode<Object> value, Label* if_number,
                                           TVariable<Word32T>* var_word32,
                                           Label* if_bigint,
-                                          TVariable<Object>* var_maybe_bigint,
+                                          TVariable<BigInt>* var_maybe_bigint,
                                           TVariable<Smi>* var_feedback);
 
   // Truncate the floating point value of a HeapNumber to an Int32.
@@ -2379,11 +2379,11 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
   TNode<Uint32T> ChangeNumberToUint32(TNode<Number> value);
   TNode<Float64T> ChangeNumberToFloat64(TNode<Number> value);
 
-  void TaggedToNumeric(TNode<Context> context, TNode<Object> value, Label* done,
-                       Variable* var_numeric);
+  void TaggedToNumeric(TNode<Context> context, TNode<Object> value,
+                       TVariable<Numeric>* var_numeric);
   void TaggedToNumericWithFeedback(TNode<Context> context, TNode<Object> value,
-                                   Label* done, Variable* var_numeric,
-                                   Variable* var_feedback);
+                                   TVariable<Numeric>* var_numeric,
+                                   TVariable<Smi>* var_feedback);
 
   TNode<WordT> TimesSystemPointerSize(SloppyTNode<WordT> value);
   TNode<IntPtrT> TimesSystemPointerSize(TNode<IntPtrT> value) {
@@ -3292,12 +3292,12 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
 
   // Combine the new feedback with the existing_feedback. Do nothing if
   // existing_feedback is nullptr.
-  void CombineFeedback(Variable* existing_feedback, int feedback);
-  void CombineFeedback(Variable* existing_feedback, Node* feedback);
+  void CombineFeedback(TVariable<Smi>* existing_feedback, int feedback);
+  void CombineFeedback(TVariable<Smi>* existing_feedback, TNode<Smi> feedback);
 
   // Overwrite the existing feedback with new_feedback. Do nothing if
   // existing_feedback is nullptr.
-  void OverwriteFeedback(Variable* existing_feedback, int new_feedback);
+  void OverwriteFeedback(TVariable<Smi>* existing_feedback, int new_feedback);
 
   // Check if a property name might require protector invalidation when it is
   // used for a property store or deletion.
@@ -3847,7 +3847,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
   TNode<Smi> CollectFeedbackForString(SloppyTNode<Int32T> instance_type);
   void GenerateEqual_Same(SloppyTNode<Object> value, Label* if_equal,
                           Label* if_notequal,
-                          Variable* var_type_feedback = nullptr);
+                          TVariable<Smi>* var_type_feedback = nullptr);
 
   static const int kElementLoopUnrollThreshold = 8;
 
@@ -3856,15 +3856,16 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
       TNode<Context> context, TNode<HeapObject> input, Object::Conversion mode,
       BigIntHandling bigint_handling = BigIntHandling::kThrow);
 
-  void TaggedToNumeric(TNode<Context> context, TNode<Object> value, Label* done,
-                       Variable* var_numeric, Variable* var_feedback);
+  void TaggedToNumeric(TNode<Context> context, TNode<Object> value,
+                       TVariable<Numeric>* var_numeric,
+                       TVariable<Smi>* var_feedback);
 
   template <Object::Conversion conversion>
   void TaggedToWord32OrBigIntImpl(TNode<Context> context, TNode<Object> value,
                                   Label* if_number,
                                   TVariable<Word32T>* var_word32,
                                   Label* if_bigint = nullptr,
-                                  TVariable<Object>* var_maybe_bigint = nullptr,
+                                  TVariable<BigInt>* var_maybe_bigint = nullptr,
                                   TVariable<Smi>* var_feedback = nullptr);
 
   Node* LoadObjectField(SloppyTNode<HeapObject> object, int offset,

src/ic/binary-op-assembler.cc

@@ -563,5 +563,85 @@ TNode<Object> BinaryOpAssembler::Generate_ExponentiateWithFeedback(
   return CallBuiltin(Builtins::kExponentiate, context, base, exponent);
 }
 
+TNode<Object> BinaryOpAssembler::Generate_BitwiseBinaryOpWithOptionalFeedback(
+    Operation bitwise_op, TNode<Object> left, TNode<Object> right,
+    TNode<Context> context, TVariable<Smi>* feedback) {
+  TVARIABLE(Object, result);
+  TVARIABLE(Smi, var_left_feedback);
+  TVARIABLE(Smi, var_right_feedback);
+  TVARIABLE(Word32T, var_left_word32);
+  TVARIABLE(Word32T, var_right_word32);
+  TVARIABLE(BigInt, var_left_bigint);
+  TVARIABLE(BigInt, var_right_bigint);
+  // These are the variables that are passed to BigIntBinaryOp. They are not
+  // guaranteed to be BigInts because the Runtime call handles throwing
+  // exceptions when only one side is a BigInt.
+  TVARIABLE(Object, var_left_maybe_bigint, left);
+  TVARIABLE(Numeric, var_right_maybe_bigint);
+  Label done(this);
+  Label if_left_number(this), do_number_op(this);
+  Label if_left_bigint(this), do_bigint_op(this);
+
+  TaggedToWord32OrBigIntWithFeedback(
+      context, left, &if_left_number, &var_left_word32, &if_left_bigint,
+      &var_left_bigint, feedback ? &var_left_feedback : nullptr);
+
+  Label right_is_bigint(this);
+  BIND(&if_left_number);
+  {
+    TaggedToWord32OrBigIntWithFeedback(
+        context, right, &do_number_op, &var_right_word32, &right_is_bigint,
+        &var_right_bigint, feedback ? &var_right_feedback : nullptr);
+  }
+
+  BIND(&right_is_bigint);
+  {
+    // At this point it's guaranteed that the op will fail because the RHS is a
+    // BigInt while the LHS is not, but that's ok because the Runtime call will
+    // throw the exception.
+    var_right_maybe_bigint = var_right_bigint.value();
+    Goto(&do_bigint_op);
+  }
+
+  BIND(&do_number_op);
+  {
+    result = BitwiseOp(var_left_word32.value(), var_right_word32.value(),
+                       bitwise_op);
+
+    if (feedback) {
+      TNode<Smi> result_type = SelectSmiConstant(
+          TaggedIsSmi(result.value()), BinaryOperationFeedback::kSignedSmall,
+          BinaryOperationFeedback::kNumber);
+      TNode<Smi> input_feedback =
+          SmiOr(var_left_feedback.value(), var_right_feedback.value());
+      *feedback = SmiOr(result_type, input_feedback);
+    }
+    Goto(&done);
+  }
+
+  // BigInt cases.
+  BIND(&if_left_bigint);
+  {
+    TaggedToNumericWithFeedback(context, right, &var_right_maybe_bigint,
+                                &var_right_feedback);
+    var_left_maybe_bigint = var_left_bigint.value();
+    Goto(&do_bigint_op);
+  }
+
+  BIND(&do_bigint_op);
+  {
+    if (feedback) {
+      *feedback = SmiOr(var_left_feedback.value(), var_right_feedback.value());
+    }
+    result = CallRuntime(
+        Runtime::kBigIntBinaryOp, context, var_left_maybe_bigint.value(),
+        var_right_maybe_bigint.value(), SmiConstant(bitwise_op));
+    Goto(&done);
+  }
+
+  BIND(&done);
+  return result.value();
+}
+
 }  // namespace internal
 }  // namespace v8

src/ic/binary-op-assembler.h

@@ -50,6 +50,23 @@ class BinaryOpAssembler : public CodeStubAssembler {
       TNode<UintPtrT> slot, TNode<HeapObject> maybe_feedback_vector,
       bool rhs_known_smi);
 
+  TNode<Object> Generate_BitwiseBinaryOpWithFeedback(Operation bitwise_op,
+                                                     TNode<Object> left,
+                                                     TNode<Object> right,
+                                                     TNode<Context> context,
+                                                     TVariable<Smi>* feedback) {
+    return Generate_BitwiseBinaryOpWithOptionalFeedback(bitwise_op, left, right,
+                                                        context, feedback);
+  }
+
+  TNode<Object> Generate_BitwiseBinaryOp(Operation bitwise_op,
+                                         TNode<Object> left,
+                                         TNode<Object> right,
+                                         TNode<Context> context) {
+    return Generate_BitwiseBinaryOpWithOptionalFeedback(bitwise_op, left, right,
+                                                        context, nullptr);
+  }
+
  private:
   using SmiOperation =
       std::function<TNode<Object>(TNode<Smi>, TNode<Smi>, TVariable<Smi>*)>;
@@ -61,6 +78,10 @@ class BinaryOpAssembler : public CodeStubAssembler {
       TNode<UintPtrT> slot, TNode<HeapObject> maybe_feedback_vector,
       const SmiOperation& smiOperation, const FloatOperation& floatOperation,
       Operation op, bool rhs_known_smi);
+
+  TNode<Object> Generate_BitwiseBinaryOpWithOptionalFeedback(
+      Operation bitwise_op, TNode<Object> left, TNode<Object> right,
+      TNode<Context> context, TVariable<Smi>* feedback);
 };
 
 }  // namespace internal

src/interpreter/interpreter-generator.cc

@@ -972,46 +972,14 @@ class InterpreterBitwiseBinaryOpAssembler : public InterpreterAssembler {
     TNode<UintPtrT> slot_index = BytecodeOperandIdx(1);
     TNode<HeapObject> maybe_feedback_vector = LoadFeedbackVector();
 
-    TVARIABLE(Smi, var_left_feedback);
-    TVARIABLE(Smi, var_right_feedback);
-    TVARIABLE(Word32T, var_left_word32);
-    TVARIABLE(Word32T, var_right_word32);
-    TVARIABLE(Object, var_left_bigint, left);
-    TVARIABLE(Object, var_right_bigint);
-    Label if_left_number(this), do_number_op(this);
-    Label if_left_bigint(this), do_bigint_op(this);
-
-    TaggedToWord32OrBigIntWithFeedback(context, left, &if_left_number,
-                                       &var_left_word32, &if_left_bigint,
-                                       &var_left_bigint, &var_left_feedback);
-    BIND(&if_left_number);
-    TaggedToWord32OrBigIntWithFeedback(context, right, &do_number_op,
-                                       &var_right_word32, &do_bigint_op,
-                                       &var_right_bigint, &var_right_feedback);
-    BIND(&do_number_op);
-    TNode<Number> result = BitwiseOp(var_left_word32.value(),
-                                     var_right_word32.value(), bitwise_op);
-    TNode<Smi> result_type = SelectSmiConstant(
-        TaggedIsSmi(result), BinaryOperationFeedback::kSignedSmall,
-        BinaryOperationFeedback::kNumber);
-    TNode<Smi> input_feedback =
-        SmiOr(var_left_feedback.value(), var_right_feedback.value());
-    UpdateFeedback(SmiOr(result_type, input_feedback), maybe_feedback_vector,
-                   slot_index);
-    SetAccumulator(result);
-    Dispatch();
+    TVARIABLE(Smi, feedback);
 
-    // BigInt cases.
-    BIND(&if_left_bigint);
-    TaggedToNumericWithFeedback(context, right, &do_bigint_op,
-                                &var_right_bigint, &var_right_feedback);
+    BinaryOpAssembler binop_asm(state());
+    TNode<Object> result = binop_asm.Generate_BitwiseBinaryOpWithFeedback(
+        bitwise_op, left, right, context, &feedback);
 
-    BIND(&do_bigint_op);
-    SetAccumulator(
-        CallRuntime(Runtime::kBigIntBinaryOp, context, var_left_bigint.value(),
-                    var_right_bigint.value(), SmiConstant(bitwise_op)));
-    UpdateFeedback(SmiOr(var_left_feedback.value(), var_right_feedback.value()),
-                   maybe_feedback_vector, slot_index);
+    UpdateFeedback(feedback.value(), maybe_feedback_vector, slot_index);
+    SetAccumulator(result);
     Dispatch();
   }
 
@@ -1024,9 +992,7 @@ class InterpreterBitwiseBinaryOpAssembler : public InterpreterAssembler {
 
     TVARIABLE(Smi, var_left_feedback);
     TVARIABLE(Word32T, var_left_word32);
-    // TODO(v8:6949): var_left_bigint should be BigInt, but before that we need
-    // to clean up TaggedToWord32OrBigIntWithFeedback and related methods.
-    TVARIABLE(Object, var_left_bigint);
+    TVARIABLE(BigInt, var_left_bigint);
     Label do_smi_op(this), if_bigint_mix(this);
 
     TaggedToWord32OrBigIntWithFeedback(context, left, &do_smi_op,
@@ -1133,9 +1099,7 @@ IGNITION_HANDLER(BitwiseNot, InterpreterAssembler) {
 
   TVARIABLE(Word32T, var_word32);
   TVARIABLE(Smi, var_feedback);
-  // TODO(v8:6949): var_bigint should be BigInt, but before that we need to
-  // clean up TaggedToWord32OrBigIntWithFeedback and related methods.
-  TVARIABLE(Object, var_bigint);
+  TVARIABLE(BigInt, var_bigint);
   Label if_number(this), if_bigint(this);
   TaggedToWord32OrBigIntWithFeedback(context, operand, &if_number, &var_word32,
                                      &if_bigint, &var_bigint, &var_feedback);