[ic] Ensure state of bitwise binary operation always progresses

This CL fixes a deopt loop that might happen in case of mixing
Number with BigInt in bitwise binary operations.

Bug: v8:12693
Change-Id: Ib6a08d0c74a954ade3719bd6bd49ca2988d88e69
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3505542Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Jianxiao Lu <jianxiao.lu@intel.com>
Cr-Commit-Position: refs/heads/main@{#79456}

src/ic/binary-op-assembler.cc

@@ -596,7 +596,9 @@ TNode<Object> BinaryOpAssembler::Generate_ExponentiateWithFeedback(
 
 TNode<Object> BinaryOpAssembler::Generate_BitwiseBinaryOpWithOptionalFeedback(
     Operation bitwise_op, TNode<Object> left, TNode<Object> right,
-    const LazyNode<Context>& context, TVariable<Smi>* feedback) {
+    const LazyNode<Context>& context, TNode<UintPtrT>* slot,
+    const LazyNode<HeapObject>* maybe_feedback_vector,
+    UpdateFeedbackMode update_feedback_mode) {
   TVARIABLE(Object, result);
   TVARIABLE(Smi, var_left_feedback);
   TVARIABLE(Smi, var_right_feedback);
@@ -615,14 +617,14 @@ TNode<Object> BinaryOpAssembler::Generate_BitwiseBinaryOpWithOptionalFeedback(
 
   TaggedToWord32OrBigIntWithFeedback(
       context(), left, &if_left_number, &var_left_word32, &if_left_bigint,
-      &var_left_bigint, feedback ? &var_left_feedback : nullptr);
+      &var_left_bigint, slot ? &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);
+        &var_right_bigint, slot ? &var_right_feedback : nullptr);
   }
 
   BIND(&right_is_bigint);
@@ -639,13 +641,15 @@ TNode<Object> BinaryOpAssembler::Generate_BitwiseBinaryOpWithOptionalFeedback(
     result = BitwiseOp(var_left_word32.value(), var_right_word32.value(),
                        bitwise_op);
 
-    if (feedback) {
+    if (slot) {
       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);
+      TNode<Smi> feedback = SmiOr(result_type, input_feedback);
+      UpdateFeedback(feedback, (*maybe_feedback_vector)(), *slot,
+                     update_feedback_mode);
     }
     Goto(&done);
   }
@@ -661,9 +665,15 @@ TNode<Object> BinaryOpAssembler::Generate_BitwiseBinaryOpWithOptionalFeedback(
 
   BIND(&do_bigint_op);
   {
-    if (feedback) {
-      *feedback = SmiOr(var_left_feedback.value(), var_right_feedback.value());
+    if (slot) {
+      // Ensure that the feedback is updated even if the runtime call below
+      // would throw.
+      TNode<Smi> feedback =
+          SmiOr(var_left_feedback.value(), var_right_feedback.value());
+      UpdateFeedback(feedback, (*maybe_feedback_vector)(), *slot,
+                     update_feedback_mode);
     }
+
     result = CallRuntime(
         Runtime::kBigIntBinaryOp, context(), var_left_maybe_bigint.value(),
         var_right_maybe_bigint.value(), SmiConstant(bitwise_op));
@@ -677,12 +687,15 @@ TNode<Object> BinaryOpAssembler::Generate_BitwiseBinaryOpWithOptionalFeedback(
 TNode<Object>
 BinaryOpAssembler::Generate_BitwiseBinaryOpWithSmiOperandAndOptionalFeedback(
     Operation bitwise_op, TNode<Object> left, TNode<Object> right,
-    const LazyNode<Context>& context, TVariable<Smi>* feedback) {
+    const LazyNode<Context>& context, TNode<UintPtrT>* slot,
+    const LazyNode<HeapObject>* maybe_feedback_vector,
+    UpdateFeedbackMode update_feedback_mode) {
   TNode<Smi> right_smi = CAST(right);
   TVARIABLE(Object, result);
   TVARIABLE(Smi, var_left_feedback);
   TVARIABLE(Word32T, var_left_word32);
   TVARIABLE(BigInt, var_left_bigint);
+  TVARIABLE(Smi, feedback);
   // Check if the {lhs} is a Smi or a HeapObject.
   Label if_lhsissmi(this), if_lhsisnotsmi(this, Label::kDeferred);
   Label do_number_op(this), if_bigint_mix(this), done(this);
@@ -693,13 +706,13 @@ BinaryOpAssembler::Generate_BitwiseBinaryOpWithSmiOperandAndOptionalFeedback(
   {
     TNode<Smi> left_smi = CAST(left);
     result = BitwiseSmiOp(left_smi, right_smi, bitwise_op);
-    if (feedback) {
+    if (slot) {
       if (IsBitwiseOutputKnownSmi(bitwise_op)) {
-        *feedback = SmiConstant(BinaryOperationFeedback::kSignedSmall);
+        feedback = SmiConstant(BinaryOperationFeedback::kSignedSmall);
       } else {
-        *feedback = SelectSmiConstant(TaggedIsSmi(result.value()),
-                                      BinaryOperationFeedback::kSignedSmall,
-                                      BinaryOperationFeedback::kNumber);
+        feedback = SelectSmiConstant(TaggedIsSmi(result.value()),
+                                     BinaryOperationFeedback::kSignedSmall,
+                                     BinaryOperationFeedback::kNumber);
       }
     }
     Goto(&done);
@@ -715,25 +728,30 @@ BinaryOpAssembler::Generate_BitwiseBinaryOpWithSmiOperandAndOptionalFeedback(
     {
       result =
           BitwiseOp(var_left_word32.value(), SmiToInt32(right_smi), bitwise_op);
-      if (feedback) {
+      if (slot) {
         TNode<Smi> result_type = SelectSmiConstant(
             TaggedIsSmi(result.value()), BinaryOperationFeedback::kSignedSmall,
             BinaryOperationFeedback::kNumber);
-        *feedback = SmiOr(result_type, var_left_feedback.value());
+        feedback = SmiOr(result_type, var_left_feedback.value());
       }
       Goto(&done);
     }
 
     BIND(&if_bigint_mix);
     {
-      if (feedback) {
-        *feedback = var_left_feedback.value();
+      if (slot) {
+        // Ensure that the feedback is updated before we throw.
+        feedback = var_left_feedback.value();
+        UpdateFeedback(feedback.value(), (*maybe_feedback_vector)(), *slot,
+                       update_feedback_mode);
       }
       ThrowTypeError(context(), MessageTemplate::kBigIntMixedTypes);
     }
   }
 
   BIND(&done);
+  UpdateFeedback(feedback.value(), (*maybe_feedback_vector)(), *slot,
+                 update_feedback_mode);
   return result.value();
 }
 

src/ic/binary-op-assembler.h

@@ -6,6 +6,7 @@
 #define V8_IC_BINARY_OP_ASSEMBLER_H_
 
 #include <functional>
+
 #include "src/codegen/code-stub-assembler.h"
 
 namespace v8 {
@@ -57,11 +58,9 @@ class BinaryOpAssembler : public CodeStubAssembler {
       const LazyNode<Context>& context, TNode<Object> left, TNode<Object> right,
       TNode<UintPtrT> slot, const LazyNode<HeapObject>& maybe_feedback_vector,
       UpdateFeedbackMode update_feedback_mode, bool rhs_known_smi) {
-    TVARIABLE(Smi, feedback);
     TNode<Object> result = Generate_BitwiseBinaryOpWithFeedback(
-        Operation::kBitwiseOr, left, right, context, &feedback, rhs_known_smi);
-    UpdateFeedback(feedback.value(), maybe_feedback_vector(), slot,
-                   update_feedback_mode);
+        Operation::kBitwiseOr, left, right, context, slot,
+        maybe_feedback_vector, update_feedback_mode, rhs_known_smi);
     return result;
   }
 
@@ -69,11 +68,10 @@ class BinaryOpAssembler : public CodeStubAssembler {
       const LazyNode<Context>& context, TNode<Object> left, TNode<Object> right,
       TNode<UintPtrT> slot, const LazyNode<HeapObject>& maybe_feedback_vector,
       UpdateFeedbackMode update_feedback_mode, bool rhs_known_smi) {
-    TVARIABLE(Smi, feedback);
     TNode<Object> result = Generate_BitwiseBinaryOpWithFeedback(
-        Operation::kBitwiseXor, left, right, context, &feedback, rhs_known_smi);
-    UpdateFeedback(feedback.value(), maybe_feedback_vector(), slot,
-                   update_feedback_mode);
+        Operation::kBitwiseXor, left, right, context, slot,
+        maybe_feedback_vector, update_feedback_mode, rhs_known_smi);
+
     return result;
   }
 
@@ -81,11 +79,10 @@ class BinaryOpAssembler : public CodeStubAssembler {
       const LazyNode<Context>& context, TNode<Object> left, TNode<Object> right,
       TNode<UintPtrT> slot, const LazyNode<HeapObject>& maybe_feedback_vector,
       UpdateFeedbackMode update_feedback_mode, bool rhs_known_smi) {
-    TVARIABLE(Smi, feedback);
     TNode<Object> result = Generate_BitwiseBinaryOpWithFeedback(
-        Operation::kBitwiseAnd, left, right, context, &feedback, rhs_known_smi);
-    UpdateFeedback(feedback.value(), maybe_feedback_vector(), slot,
-                   update_feedback_mode);
+        Operation::kBitwiseAnd, left, right, context, slot,
+        maybe_feedback_vector, update_feedback_mode, rhs_known_smi);
+
     return result;
   }
 
@@ -93,11 +90,10 @@ class BinaryOpAssembler : public CodeStubAssembler {
       const LazyNode<Context>& context, TNode<Object> left, TNode<Object> right,
       TNode<UintPtrT> slot, const LazyNode<HeapObject>& maybe_feedback_vector,
       UpdateFeedbackMode update_feedback_mode, bool rhs_known_smi) {
-    TVARIABLE(Smi, feedback);
     TNode<Object> result = Generate_BitwiseBinaryOpWithFeedback(
-        Operation::kShiftLeft, left, right, context, &feedback, rhs_known_smi);
-    UpdateFeedback(feedback.value(), maybe_feedback_vector(), slot,
-                   update_feedback_mode);
+        Operation::kShiftLeft, left, right, context, slot,
+        maybe_feedback_vector, update_feedback_mode, rhs_known_smi);
+
     return result;
   }
 
@@ -105,11 +101,10 @@ class BinaryOpAssembler : public CodeStubAssembler {
       const LazyNode<Context>& context, TNode<Object> left, TNode<Object> right,
       TNode<UintPtrT> slot, const LazyNode<HeapObject>& maybe_feedback_vector,
       UpdateFeedbackMode update_feedback_mode, bool rhs_known_smi) {
-    TVARIABLE(Smi, feedback);
     TNode<Object> result = Generate_BitwiseBinaryOpWithFeedback(
-        Operation::kShiftRight, left, right, context, &feedback, rhs_known_smi);
-    UpdateFeedback(feedback.value(), maybe_feedback_vector(), slot,
-                   update_feedback_mode);
+        Operation::kShiftRight, left, right, context, slot,
+        maybe_feedback_vector, update_feedback_mode, rhs_known_smi);
+
     return result;
   }
 
@@ -117,24 +112,25 @@ class BinaryOpAssembler : public CodeStubAssembler {
       const LazyNode<Context>& context, TNode<Object> left, TNode<Object> right,
       TNode<UintPtrT> slot, const LazyNode<HeapObject>& maybe_feedback_vector,
       UpdateFeedbackMode update_feedback_mode, bool rhs_known_smi) {
-    TVARIABLE(Smi, feedback);
     TNode<Object> result = Generate_BitwiseBinaryOpWithFeedback(
-        Operation::kShiftRightLogical, left, right, context, &feedback,
-        rhs_known_smi);
-    UpdateFeedback(feedback.value(), maybe_feedback_vector(), slot,
-                   update_feedback_mode);
+        Operation::kShiftRightLogical, left, right, context, slot,
+        maybe_feedback_vector, update_feedback_mode, rhs_known_smi);
+
     return result;
   }
 
   TNode<Object> Generate_BitwiseBinaryOpWithFeedback(
       Operation bitwise_op, TNode<Object> left, TNode<Object> right,
-      const LazyNode<Context>& context, TVariable<Smi>* feedback,
-      bool rhs_known_smi) {
+      const LazyNode<Context>& context, TNode<UintPtrT> slot,
+      const LazyNode<HeapObject>& maybe_feedback_vector,
+      UpdateFeedbackMode update_feedback_mode, bool rhs_known_smi) {
     return rhs_known_smi
                ? Generate_BitwiseBinaryOpWithSmiOperandAndOptionalFeedback(
-                     bitwise_op, left, right, context, feedback)
+                     bitwise_op, left, right, context, &slot,
+                     &maybe_feedback_vector, update_feedback_mode)
                : Generate_BitwiseBinaryOpWithOptionalFeedback(
-                     bitwise_op, left, right, context, feedback);
+                     bitwise_op, left, right, context, &slot,
+                     &maybe_feedback_vector, update_feedback_mode);
   }
 
   TNode<Object> Generate_BitwiseBinaryOp(Operation bitwise_op,
@@ -142,7 +138,8 @@ class BinaryOpAssembler : public CodeStubAssembler {
                                          TNode<Object> right,
                                          TNode<Context> context) {
     return Generate_BitwiseBinaryOpWithOptionalFeedback(
-        bitwise_op, left, right, [&] { return context; }, nullptr);
+        bitwise_op, left, right, [&] { return context; }, nullptr, nullptr,
+        UpdateFeedbackMode::kOptionalFeedback);
   }
 
  private:
@@ -160,11 +157,15 @@ class BinaryOpAssembler : public CodeStubAssembler {
 
   TNode<Object> Generate_BitwiseBinaryOpWithOptionalFeedback(
       Operation bitwise_op, TNode<Object> left, TNode<Object> right,
-      const LazyNode<Context>& context, TVariable<Smi>* feedback);
+      const LazyNode<Context>& context, TNode<UintPtrT>* slot,
+      const LazyNode<HeapObject>* maybe_feedback_vector,
+      UpdateFeedbackMode update_feedback_mode);
 
   TNode<Object> Generate_BitwiseBinaryOpWithSmiOperandAndOptionalFeedback(
       Operation bitwise_op, TNode<Object> left, TNode<Object> right,
-      const LazyNode<Context>& context, TVariable<Smi>* feedback);
+      const LazyNode<Context>& context, TNode<UintPtrT>* slot,
+      const LazyNode<HeapObject>* maybe_feedback_vector,
+      UpdateFeedbackMode update_feedback_mode);
 
   // Check if output is known to be Smi when both operands of bitwise operation
   // are Smi.

src/interpreter/interpreter-generator.cc

@@ -1007,13 +1007,12 @@ class InterpreterBitwiseBinaryOpAssembler : public InterpreterAssembler {
     TNode<UintPtrT> slot_index = BytecodeOperandIdx(1);
     TNode<HeapObject> maybe_feedback_vector = LoadFeedbackVector();
 
-    TVARIABLE(Smi, feedback);
-
     BinaryOpAssembler binop_asm(state());
     TNode<Object> result = binop_asm.Generate_BitwiseBinaryOpWithFeedback(
-        bitwise_op, left, right, [=] { return context; }, &feedback, false);
+        bitwise_op, left, right, [=] { return context; }, slot_index,
+        [=] { return maybe_feedback_vector; },
+        UpdateFeedbackMode::kOptionalFeedback, false);
 
-    MaybeUpdateFeedback(feedback.value(), maybe_feedback_vector, slot_index);
     SetAccumulator(result);
     Dispatch();
   }
@@ -1025,13 +1024,12 @@ class InterpreterBitwiseBinaryOpAssembler : public InterpreterAssembler {
     TNode<HeapObject> maybe_feedback_vector = LoadFeedbackVector();
     TNode<Context> context = GetContext();
 
-    TVARIABLE(Smi, feedback);
-
     BinaryOpAssembler binop_asm(state());
     TNode<Object> result = binop_asm.Generate_BitwiseBinaryOpWithFeedback(
-        bitwise_op, left, right, [=] { return context; }, &feedback, true);
+        bitwise_op, left, right, [=] { return context; }, slot_index,
+        [=] { return maybe_feedback_vector; },
+        UpdateFeedbackMode::kOptionalFeedback, true);
 
-    MaybeUpdateFeedback(feedback.value(), maybe_feedback_vector, slot_index);
     SetAccumulator(result);
     Dispatch();
   }
@@ -3092,8 +3090,7 @@ Handle<Code> GenerateBytecodeHandler(Isolate* isolate, const char* debug_name,
   Zone zone(isolate->allocator(), ZONE_NAME, kCompressGraphZone);
   compiler::CodeAssemblerState state(
       isolate, &zone, InterpreterDispatchDescriptor{},
-      CodeKind::BYTECODE_HANDLER, debug_name,
-      builtin);
+      CodeKind::BYTECODE_HANDLER, debug_name, builtin);
 
   switch (bytecode) {
 #define CALL_GENERATOR(Name, ...)                     \

test/mjsunit/regress/regress-binary-bitwise-bigint-smi-mix-opt-depot.js

+// Copyright 2022 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --opt --allow-natives-syntax --no-always-opt
+
+function foo(n) {
+  let v = 0;
+  for (let i = 0n; i < n; ++i) {
+    v = v | 1;
+    v = i;
+  }
+
+  v = 0;
+  for (let i = 0n; i < n; ++i) {
+    v = v ^ 1;
+    v = i;
+  }
+
+  v = 0;
+  for (let i = 0n; i < n; ++i) {
+    v = v & 1;
+    v = i;
+  }
+
+  v = 0;
+  for (let i = 0n; i < n; ++i) {
+    v = v << 1;
+    v = i;
+  }
+
+  v = 0;
+  for (let i = 0n; i < n; ++i) {
+    v = v >> 1;
+    v = i;
+  }
+
+  v = 0;
+  for (let i = 0n; i < n; ++i) {
+    v = v >>> 1;
+    v = i;
+  }
+}
+
+%PrepareFunctionForOptimization(foo);
+assertDoesNotThrow(() => foo(1n));
+%OptimizeFunctionOnNextCall(foo);
+assertDoesNotThrow(() => foo(1n));
+assertOptimized(foo);
+%PrepareFunctionForOptimization(foo);
+assertThrows(() => foo(2n), TypeError);
+%OptimizeFunctionOnNextCall(foo);
+assertDoesNotThrow(() => foo(1n));
+assertOptimized(foo);
+assertThrows(() => foo(2n), TypeError);
+assertOptimized(foo);