[cleanup] Consolidate ToWord32/ToNumeric helpers

as well as "BitwiseOp". Builtins and Interpreter bytecode handlers need
quite a bit of similar functionality with minor differences.
This CL factors out and generalizes the TaggedToNumeric[WithFeedback]
and the TaggedToWord[OrBigInt][WithFeedback] groups of helpers into one
shared implementation each in the CodeStubAssembler.

Bug: v8:6921
Change-Id: Iae5dcc4c50c7fde3423f801cb5484de337381ce6
Reviewed-on: https://chromium-review.googlesource.com/721606
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48798}

src/builtins/builtins-number-gen.cc

@@ -21,58 +21,29 @@ class NumberBuiltinsAssembler : public CodeStubAssembler {
 
  protected:
   template <typename Descriptor>
-  void BitwiseOp(Token::Value op) {
+  void EmitBitwiseOp(Token::Value op) {
     Node* left = Parameter(Descriptor::kLeft);
     Node* right = Parameter(Descriptor::kRight);
     Node* context = Parameter(Descriptor::kContext);
 
+    VARIABLE(var_left_word32, MachineRepresentation::kWord32);
+    VARIABLE(var_right_word32, MachineRepresentation::kWord32);
     VARIABLE(var_left_bigint, MachineRepresentation::kTagged, left);
-    VARIABLE(var_right_bigint, MachineRepresentation::kTagged, right);
+    VARIABLE(var_right_bigint, MachineRepresentation::kTagged);
+    Label if_left_number(this), do_number_op(this);
     Label if_left_bigint(this), do_bigint_op(this);
 
-    Node* left32 = TaggedToWord32OrBigInt(context, left, &if_left_bigint,
-                                          &var_left_bigint);
-    Node* right32 = TaggedToWord32OrBigInt(context, right, &do_bigint_op,
-                                           &var_right_bigint);
-    // Number case.
-    Node* result;
-    switch (op) {
-      case Token::BIT_AND:
-        result = ChangeInt32ToTagged(Signed(Word32And(left32, right32)));
-        break;
-      case Token::BIT_OR:
-        result = ChangeInt32ToTagged(Signed(Word32Or(left32, right32)));
-        break;
-      case Token::BIT_XOR:
-        result = ChangeInt32ToTagged(Signed(Word32Xor(left32, right32)));
-        break;
-      case Token::SHL:
-        result = ChangeInt32ToTagged(
-            Signed(Word32Shl(left32, Word32And(right32, Int32Constant(0x1f)))));
-        break;
-      case Token::SAR:
-        result = ChangeInt32ToTagged(
-            Signed(Word32Sar(left32, Word32And(right32, Int32Constant(0x1f)))));
-        break;
-      case Token::SHR:
-        result = ChangeUint32ToTagged(Unsigned(
-            Word32Shr(left32, Word32And(right32, Int32Constant(0x1f)))));
-        break;
-      default:
-        UNREACHABLE();
-    }
-    Return(result);
+    TaggedToWord32OrBigInt(context, left, &if_left_number, &var_left_word32,
+                           &if_left_bigint, &var_left_bigint);
+    BIND(&if_left_number);
+    TaggedToWord32OrBigInt(context, right, &do_number_op, &var_right_word32,
+                           &do_bigint_op, &var_right_bigint);
+    BIND(&do_number_op);
+    Return(BitwiseOp(var_left_word32.value(), var_right_word32.value(), op));
 
     // BigInt cases.
     BIND(&if_left_bigint);
-    GotoIf(TaggedIsSmi(right), &do_bigint_op);
-    Node* right_map = LoadMap(right);
-    GotoIf(IsHeapNumberMap(right_map), &do_bigint_op);
-    Node* right_type = LoadMapInstanceType(right_map);
-    GotoIf(IsBigIntInstanceType(right_type), &do_bigint_op);
-    var_right_bigint.Bind(
-        CallBuiltin(Builtins::kNonNumberToNumeric, context, right));
-    Goto(&do_bigint_op);
+    TaggedToNumeric(context, right, &do_bigint_op, &var_right_bigint);
 
     BIND(&do_bigint_op);
     Return(CallRuntime(Runtime::kBigIntBinaryOp, context,
@@ -93,47 +64,6 @@ class NumberBuiltinsAssembler : public CodeStubAssembler {
   void BinaryOp(Label* smis, Variable* var_left, Variable* var_right,
                 Label* doubles, Variable* var_left_double,
                 Variable* var_right_double, Label* bigints);
-
-  // Similar to CodeStubAssembler::TruncateTaggedToWord32, but BigInt aware.
-  // Jumps to {bigint} with {var_bigint_result} populated if it encounters
-  // a BigInt.
-  Node* TaggedToWord32OrBigInt(Node* context, Node* value, Label* bigint,
-                               Variable* var_bigint_result) {
-    // We might need to loop once due to ToNumeric conversion.
-    VARIABLE(var_value, MachineRepresentation::kTagged, value);
-    VARIABLE(var_result, MachineRepresentation::kWord32);
-    Label loop(this, &var_value), done(this, &var_result);
-    Goto(&loop);
-    BIND(&loop);
-    {
-      value = var_value.value();
-      Label not_smi(this), is_heap_number(this), is_bigint(this);
-      GotoIf(TaggedIsNotSmi(value), &not_smi);
-      // {value} is a Smi.
-      var_result.Bind(SmiToWord32(value));
-      Goto(&done);
-
-      BIND(&not_smi);
-      Node* map = LoadMap(value);
-      GotoIf(IsHeapNumberMap(map), &is_heap_number);
-      Node* instance_type = LoadMapInstanceType(map);
-      GotoIf(IsBigIntInstanceType(instance_type), &is_bigint);
-      // Neither HeapNumber nor BigInt -> convert to Numeric.
-      var_value.Bind(
-          CallBuiltin(Builtins::kNonNumberToNumeric, context, value));
-      Goto(&loop);
-
-      BIND(&is_heap_number);
-      var_result.Bind(TruncateHeapNumberValueToWord32(value));
-      Goto(&done);
-
-      BIND(&is_bigint);
-      var_bigint_result->Bind(value);
-      Goto(bigint);
-    }
-    BIND(&done);
-    return var_result.value();
-  }
 };
 
 // ES6 #sec-number.isfinite
@@ -920,27 +850,27 @@ TF_BUILTIN(Modulus, NumberBuiltinsAssembler) {
 }
 
 TF_BUILTIN(ShiftLeft, NumberBuiltinsAssembler) {
-  BitwiseOp<Descriptor>(Token::SHL);
+  EmitBitwiseOp<Descriptor>(Token::SHL);
 }
 
 TF_BUILTIN(ShiftRight, NumberBuiltinsAssembler) {
-  BitwiseOp<Descriptor>(Token::SAR);
+  EmitBitwiseOp<Descriptor>(Token::SAR);
 }
 
 TF_BUILTIN(ShiftRightLogical, NumberBuiltinsAssembler) {
-  BitwiseOp<Descriptor>(Token::SHR);
+  EmitBitwiseOp<Descriptor>(Token::SHR);
 }
 
 TF_BUILTIN(BitwiseAnd, NumberBuiltinsAssembler) {
-  BitwiseOp<Descriptor>(Token::BIT_AND);
+  EmitBitwiseOp<Descriptor>(Token::BIT_AND);
 }
 
 TF_BUILTIN(BitwiseOr, NumberBuiltinsAssembler) {
-  BitwiseOp<Descriptor>(Token::BIT_OR);
+  EmitBitwiseOp<Descriptor>(Token::BIT_OR);
 }
 
 TF_BUILTIN(BitwiseXor, NumberBuiltinsAssembler) {
-  BitwiseOp<Descriptor>(Token::BIT_XOR);
+  EmitBitwiseOp<Descriptor>(Token::BIT_XOR);
 }
 
 TF_BUILTIN(LessThan, NumberBuiltinsAssembler) {

src/code-stub-assembler.h

@@ -285,6 +285,8 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
   void GotoIfNotNumber(Node* value, Label* is_not_number);
   void GotoIfNumber(Node* value, Label* is_number);
 
+  Node* BitwiseOp(Node* left32, Node* right32, Token::Value bitwise_op);
+
   // Allocate an object of the given size.
   Node* AllocateInNewSpace(Node* size, AllocationFlags flags = kNone);
   Node* AllocateInNewSpace(int size, AllocationFlags flags = kNone);
@@ -944,6 +946,13 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
   Node* TryTaggedToFloat64(Node* value, Label* if_valueisnotnumber);
   Node* TruncateTaggedToFloat64(Node* context, Node* value);
   Node* TruncateTaggedToWord32(Node* context, Node* value);
+  void TaggedToWord32OrBigInt(Node* context, Node* value, Label* if_number,
+                              Variable* var_word32, Label* if_bigint,
+                              Variable* var_bigint);
+  void TaggedToWord32OrBigIntWithFeedback(
+      Node* context, Node* value, Label* if_number, Variable* var_word32,
+      Label* if_bigint, Variable* var_bigint, Variable* var_feedback);
+
   // Truncate the floating point value of a HeapNumber to an Int32.
   Node* TruncateHeapNumberValueToWord32(Node* object);
 
@@ -954,6 +963,12 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
   TNode<Float64T> ChangeNumberToFloat64(SloppyTNode<Number> value);
   TNode<UintPtrT> ChangeNonnegativeNumberToUintPtr(SloppyTNode<Number> value);
 
+  void TaggedToNumeric(Node* context, Node* value, Label* done,
+                       Variable* var_numeric);
+  void TaggedToNumericWithFeedback(Node* context, Node* value, Label* done,
+                                   Variable* var_numeric,
+                                   Variable* var_feedback);
+
   Node* TimesPointerSize(Node* value);
 
   // Type conversions.
@@ -1870,6 +1885,18 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
 
   Node* NonNumberToNumberOrNumeric(Node* context, Node* input,
                                    Object::Conversion mode);
+
+  enum class Feedback { kCollect, kNone };
+  template <Feedback feedback>
+  void TaggedToNumeric(Node* context, Node* value, Label* done,
+                       Variable* var_numeric, Variable* var_feedback = nullptr);
+
+  template <Feedback feedback, Object::Conversion conversion>
+  void TaggedToWord32OrBigIntImpl(Node* context, Node* value, Label* if_number,
+                                  Variable* var_word32,
+                                  Label* if_bigint = nullptr,
+                                  Variable* var_bigint = nullptr,
+                                  Variable* var_feedback = nullptr);
 };
 
 class CodeStubArguments {

src/interpreter/interpreter-assembler.cc

@@ -1284,76 +1284,6 @@ void InterpreterAssembler::DispatchWide(OperandScale operand_scale) {
   DispatchToBytecodeHandlerEntry(target_code_entry, next_bytecode_offset);
 }
 
-// Like NumberBuiltinsAssembler::TaggedToWord32OrBigInt, but with feedback.
-Node* InterpreterAssembler::TaggedToWord32OrBigIntWithFeedback(
-    Node* context, Node* value, Variable* var_type_feedback, Label* bigint,
-    Variable* var_bigint_result) {
-  // We might need to loop once due to ToNumeric conversion.
-  VARIABLE(var_value, MachineRepresentation::kTagged, value);
-  VARIABLE(var_result, MachineRepresentation::kWord32);
-  Variable* loop_vars[] = {&var_value, var_type_feedback};
-  Label loop(this, arraysize(loop_vars), loop_vars), done(this, &var_result);
-  var_type_feedback->Bind(SmiConstant(BinaryOperationFeedback::kNone));
-  Goto(&loop);
-  BIND(&loop);
-  {
-    value = var_value.value();
-    Label not_smi(this), is_heap_number(this), is_oddball(this),
-        is_bigint(this);
-    GotoIf(TaggedIsNotSmi(value), &not_smi);
-    // {value} is a Smi.
-    var_result.Bind(SmiToWord32(value));
-    var_type_feedback->Bind(
-        SmiOr(var_type_feedback->value(),
-              SmiConstant(BinaryOperationFeedback::kSignedSmall)));
-    Goto(&done);
-
-    BIND(&not_smi);
-    Node* map = LoadMap(value);
-    GotoIf(IsHeapNumberMap(map), &is_heap_number);
-    Node* instance_type = LoadMapInstanceType(map);
-    GotoIf(IsBigIntInstanceType(instance_type), &is_bigint);
-
-    // Not HeapNumber or BigInt.
-    {
-      // We do not require an Or with earlier feedback here because once we
-      // convert the value to a Numeric, we cannot reach this path. We can
-      // only reach this path on the first pass when the feedback is kNone.
-      CSA_ASSERT(this, SmiEqual(var_type_feedback->value(),
-                                SmiConstant(BinaryOperationFeedback::kNone)));
-      GotoIf(InstanceTypeEqual(instance_type, ODDBALL_TYPE), &is_oddball);
-
-      // Not an oddball either -> convert to Numeric.
-      var_value.Bind(
-          CallBuiltin(Builtins::kNonNumberToNumeric, context, value));
-      var_type_feedback->Bind(SmiConstant(BinaryOperationFeedback::kAny));
-      Goto(&loop);
-
-      BIND(&is_oddball);
-      var_value.Bind(LoadObjectField(value, Oddball::kToNumberOffset));
-      var_type_feedback->Bind(
-          SmiConstant(BinaryOperationFeedback::kNumberOrOddball));
-      Goto(&loop);
-    }
-
-    BIND(&is_heap_number);
-    var_result.Bind(TruncateHeapNumberValueToWord32(value));
-    var_type_feedback->Bind(
-        SmiOr(var_type_feedback->value(),
-              SmiConstant(BinaryOperationFeedback::kNumber)));
-    Goto(&done);
-
-    BIND(&is_bigint);
-    var_bigint_result->Bind(value);
-    var_type_feedback->Bind(
-        SmiOr(var_type_feedback->value(),
-              SmiConstant(BinaryOperationFeedback::kBigInt)));
-    Goto(bigint);
-  }
-  BIND(&done);
-  return var_result.value();
-}
-
 void InterpreterAssembler::UpdateInterruptBudgetOnReturn() {
   // TODO(rmcilroy): Investigate whether it is worth supporting self
   // optimization of primitive functions like FullCodegen.

src/interpreter/interpreter-assembler.h

@@ -221,12 +221,6 @@ class V8_EXPORT_PRIVATE InterpreterAssembler : public CodeStubAssembler {
   // Dispatch bytecode as wide operand variant.
   void DispatchWide(OperandScale operand_scale);
 
-  // Truncate tagged |value| to word32, or find that it is a BigInt and jump
-  // to {bigint}, and store the type feedback in |var_type_feedback|.
-  compiler::Node* TaggedToWord32OrBigIntWithFeedback(
-      compiler::Node* context, compiler::Node* value,
-      Variable* var_type_feedback, Label* bigint, Variable* var_bigint_result);
-
   // Abort with the given bailout reason.
   void Abort(BailoutReason bailout_reason);
   void AbortIfWordNotEqual(compiler::Node* lhs, compiler::Node* rhs,

src/interpreter/interpreter-generator.cc

@@ -946,17 +946,23 @@ class InterpreterBitwiseBinaryOpAssembler : public InterpreterAssembler {
 
     VARIABLE(var_left_feedback, MachineRepresentation::kTaggedSigned);
     VARIABLE(var_right_feedback, MachineRepresentation::kTaggedSigned);
+    VARIABLE(var_left_word32, MachineRepresentation::kWord32);
+    VARIABLE(var_right_word32, MachineRepresentation::kWord32);
     VARIABLE(var_left_bigint, MachineRepresentation::kTagged, left);
-    VARIABLE(var_right_bigint, MachineRepresentation::kTagged, right);
+    VARIABLE(var_right_bigint, MachineRepresentation::kTagged);
+    Label if_left_number(this), do_number_op(this);
     Label if_left_bigint(this), do_bigint_op(this);
 
-    Node* left32 = TaggedToWord32OrBigIntWithFeedback(
-        context, left, &var_left_feedback, &if_left_bigint, &var_left_bigint);
-    Node* right32 = TaggedToWord32OrBigIntWithFeedback(
-        context, right, &var_right_feedback, &do_bigint_op, &var_right_bigint);
-
-    // Numbers case.
-    Node* result = BitwiseOp(left32, right32, bitwise_op);
+    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);
+    Node* result = BitwiseOp(var_left_word32.value(), var_right_word32.value(),
+                             bitwise_op);
     Node* result_type = SelectSmiConstant(TaggedIsSmi(result),
                                           BinaryOperationFeedback::kSignedSmall,
                                           BinaryOperationFeedback::kNumber);
@@ -969,11 +975,8 @@ class InterpreterBitwiseBinaryOpAssembler : public InterpreterAssembler {
 
     // BigInt cases.
     BIND(&if_left_bigint);
-    // TODO(jkummerow): NumberBuiltinsAssembler::BitwiseOp inlines the
-    // relevant bits of this. Find a way to unify the approaches.
-    TaggedToWord32OrBigIntWithFeedback(context, right, &var_right_feedback,
-                                       &do_bigint_op, &var_right_bigint);
-    Goto(&do_bigint_op);
+    TaggedToNumericWithFeedback(context, right, &do_bigint_op,
+                                &var_right_bigint, &var_right_feedback);
 
     BIND(&do_bigint_op);
     SetAccumulator(
@@ -992,12 +995,16 @@ class InterpreterBitwiseBinaryOpAssembler : public InterpreterAssembler {
     Node* context = GetContext();
 
     VARIABLE(var_left_feedback, MachineRepresentation::kTaggedSigned);
+    VARIABLE(var_left_word32, MachineRepresentation::kWord32);
     VARIABLE(var_left_bigint, MachineRepresentation::kTagged);
-    Label if_bigint_mix(this);
+    Label do_smi_op(this), if_bigint_mix(this);
 
-    Node* left32 = TaggedToWord32OrBigIntWithFeedback(
-        context, left, &var_left_feedback, &if_bigint_mix, &var_left_bigint);
-    Node* result = BitwiseOp(left32, SmiToWord32(right), bitwise_op);
+    TaggedToWord32OrBigIntWithFeedback(context, left, &do_smi_op,
+                                       &var_left_word32, &if_bigint_mix,
+                                       &var_left_bigint, &var_left_feedback);
+    BIND(&do_smi_op);
+    Node* result =
+        BitwiseOp(var_left_word32.value(), SmiToWord32(right), bitwise_op);
     Node* result_type = SelectSmiConstant(TaggedIsSmi(result),
                                           BinaryOperationFeedback::kSignedSmall,
                                           BinaryOperationFeedback::kNumber);
@@ -1010,30 +1017,6 @@ class InterpreterBitwiseBinaryOpAssembler : public InterpreterAssembler {
     UpdateFeedback(var_left_feedback.value(), feedback_vector, slot_index);
     ThrowTypeError(context, MessageTemplate::kBigIntMixedTypes);
   }
-
- private:
-  Node* BitwiseOp(Node* left32, Node* right32, Token::Value bitwise_op) {
-    switch (bitwise_op) {
-      case Token::BIT_AND:
-        return ChangeInt32ToTagged(Signed(Word32And(left32, right32)));
-      case Token::BIT_OR:
-        return ChangeInt32ToTagged(Signed(Word32Or(left32, right32)));
-      case Token::BIT_XOR:
-        return ChangeInt32ToTagged(Signed(Word32Xor(left32, right32)));
-      case Token::SHL:
-        return ChangeInt32ToTagged(
-            Signed(Word32Shl(left32, Word32And(right32, Int32Constant(0x1f)))));
-      case Token::SAR:
-        return ChangeInt32ToTagged(
-            Signed(Word32Sar(left32, Word32And(right32, Int32Constant(0x1f)))));
-      case Token::SHR:
-        return ChangeUint32ToTagged(Unsigned(
-            Word32Shr(left32, Word32And(right32, Int32Constant(0x1f)))));
-      default:
-        break;
-    }
-    UNREACHABLE();
-  }
 };
 
 // BitwiseOr <src>
@@ -1117,15 +1100,16 @@ IGNITION_HANDLER(BitwiseNot, InterpreterAssembler) {
   Node* feedback_vector = LoadFeedbackVector();
   Node* context = GetContext();
 
+  VARIABLE(var_word32, MachineRepresentation::kWord32);
   VARIABLE(var_feedback, MachineRepresentation::kTaggedSigned);
   VARIABLE(var_bigint, MachineRepresentation::kTagged);
-  Label if_bigint(this);
-  Node* truncated_value = TaggedToWord32OrBigIntWithFeedback(
-      context, operand, &var_feedback, &if_bigint, &var_bigint);
+  Label if_number(this), if_bigint(this);
+  TaggedToWord32OrBigIntWithFeedback(context, operand, &if_number, &var_word32,
+                                     &if_bigint, &var_bigint, &var_feedback);
 
   // Number case.
-  Node* value = Word32Not(truncated_value);
-  Node* result = ChangeInt32ToTagged(value);
+  BIND(&if_number);
+  Node* result = ChangeInt32ToTagged(Signed(Word32Not(var_word32.value())));
   Node* result_type = SelectSmiConstant(TaggedIsSmi(result),
                                         BinaryOperationFeedback::kSignedSmall,
                                         BinaryOperationFeedback::kNumber);