Revert "[turbofan] Improve equality on NumberOrOddball"

This reverts commit 6204768b.

Reason for revert: A number of Clusterfuzz reports (e.g. https://bugs.chromium.org/p/chromium/issues/detail?id=1079474)

Original change's description:
> [turbofan] Improve equality on NumberOrOddball
> 
> This CL cleans up CompareOperationFeedback by replacing it with a
> composable set of flags. The interpreter is changed to collect
> more specific feedback for abstract equality, especially if oddballs
> are involved.
> 
> TurboFan is changed to construct SpeculativeNumberEqual operator
> instead of the generic JSEqual in many more cases. This change has
> shown a local speedup of a factor of 3-10, because the specific
> operator is way faster than calling into the generic builtin, but
> it also enables additional optimizations, further improving
> runtime performance.
> 
> Bug: v8:5660
> Change-Id: I856752caa707e9a4f742c6e7a9c75552fb431d28
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2162854
> Reviewed-by: Mythri Alle <mythria@chromium.org>
> Reviewed-by: Georg Neis <neis@chromium.org>
> Commit-Queue: Nico Hartmann <nicohartmann@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#67645}

TBR=rmcilroy@chromium.org,neis@chromium.org,mythria@chromium.org,nicohartmann@chromium.org

Change-Id: I3410310ed2b1ff2eaee70c1b91c3151d35866108
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:5660
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2190414Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Commit-Queue: Nico Hartmann <nicohartmann@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67673}

src/common/globals.h

@@ -1348,46 +1348,31 @@ class BinaryOperationFeedback {
 };
 
 // Type feedback is encoded in such a way that, we can combine the feedback
-// at different points by performing an 'OR' operation.
+// at different points by performing an 'OR' operation. Type feedback moves
+// to a more generic type when we combine feedback.
+//
+//   kSignedSmall -> kNumber             -> kNumberOrOddball           -> kAny
+//                   kReceiver           -> kReceiverOrNullOrUndefined -> kAny
+//                   kInternalizedString -> kString                    -> kAny
+//                                          kSymbol                    -> kAny
+//                                          kBigInt                    -> kAny
+//
 // This is distinct from BinaryOperationFeedback on purpose, because the
 // feedback that matters differs greatly as well as the way it is consumed.
 class CompareOperationFeedback {
-  enum {
-    kSignedSmallFlag = 1 << 0,
-    kOtherNumberFlag = 1 << 1,
-    kBooleanFlag = 1 << 2,
-    kOtherOddballFlag = 1 << 3,
-    kNullOrUndefinedFlag = 1 << 4,
-    kInternalizedStringFlag = 1 << 5,
-    kOtherStringFlag = 1 << 6,
-    kSymbolFlag = 1 << 7,
-    kBigIntFlag = 1 << 8,
-    kReceiverFlag = 1 << 9,
-    kAnyMask = 0x3FF,
-  };
-
  public:
-  enum Type {
-    kNone = 0,
-
-    kBoolean = kBooleanFlag,
-    kNullOrUndefined = kNullOrUndefinedFlag,
-    kOddball = kBoolean | kOtherOddballFlag | kNullOrUndefined,
-
-    kSignedSmall = kSignedSmallFlag,
-    kNumber = kSignedSmall | kOtherNumberFlag,
-    kNumberOrOddball = kNumber | kOddball,
-
-    kInternalizedString = kInternalizedStringFlag,
-    kString = kInternalizedString | kOtherStringFlag,
-
-    kReceiver = kReceiverFlag,
-    kReceiverOrNullOrUndefined = kReceiver | kNullOrUndefined,
-
-    kBigInt = kBigIntFlag,
-    kSymbol = kSymbolFlag,
-
-    kAny = kAnyMask,
+  enum {
+    kNone = 0x000,
+    kSignedSmall = 0x001,
+    kNumber = 0x003,
+    kNumberOrOddball = 0x007,
+    kInternalizedString = 0x008,
+    kString = 0x018,
+    kSymbol = 0x020,
+    kBigInt = 0x040,
+    kReceiver = 0x080,
+    kReceiverOrNullOrUndefined = 0x180,
+    kAny = 0x1ff
   };
 };
 

src/compiler/code-assembler.cc

@@ -1393,7 +1393,6 @@ void CodeAssemblerLabel::MergeVariables() {
     }
     // If the following asserts, then you've jumped to a label without a bound
     // variable along that path that expects to merge its value into a phi.
-    // This can also occur if a label is bound that is never jumped to.
     DCHECK(variable_phis_.find(var) == variable_phis_.end() ||
            count == merge_count_);
     USE(count);

src/compiler/js-typed-lowering.cc

@@ -887,12 +887,7 @@ Reduction JSTypedLowering::ReduceJSStrictEqual(Node* node) {
   if (r.BothInputsAre(Type::Signed32()) ||
       r.BothInputsAre(Type::Unsigned32())) {
     return r.ChangeToPureOperator(simplified()->NumberEqual());
-  } else if (r.GetCompareNumberOperationHint(&hint) &&
-             hint != NumberOperationHint::kNumberOrOddball) {
-    // SpeculativeNumberEqual[kNumberOrOddball] performs implicit conversion
-    // of oddballs to numbers, so we must not generate it for strict equality.
-    DCHECK(hint == NumberOperationHint::kNumber ||
-           hint == NumberOperationHint::kSignedSmall);
+  } else if (r.GetCompareNumberOperationHint(&hint)) {
     return r.ChangeToSpeculativeOperator(
         simplified()->SpeculativeNumberEqual(hint), Type::Boolean());
   } else if (r.BothInputsAre(Type::Number())) {

src/compiler/simplified-lowering.cc

@@ -2054,6 +2054,11 @@ class RepresentationSelector {
             // This doesn't make sense for compare operations.
             UNREACHABLE();
           case NumberOperationHint::kNumberOrOddball:
+            // Abstract and strict equality don't perform ToNumber conversions
+            // on Oddballs, so make sure we don't accidentially sneak in a
+            // hint with Oddball feedback here.
+            DCHECK_NE(IrOpcode::kSpeculativeNumberEqual, node->opcode());
+            V8_FALLTHROUGH;
           case NumberOperationHint::kNumber:
             VisitBinop<T>(node,
                           CheckedUseInfoAsFloat64FromHint(

src/objects/feedback-vector-inl.h

@@ -233,47 +233,32 @@ BinaryOperationHint BinaryOperationHintFromFeedback(int type_feedback) {
 }
 
 // Helper function to transform the feedback to CompareOperationHint.
-template <CompareOperationFeedback::Type Feedback>
-bool Is(int type_feedback) {
-  return !(type_feedback & ~Feedback);
-}
-
 CompareOperationHint CompareOperationHintFromFeedback(int type_feedback) {
-  if (Is<CompareOperationFeedback::kNone>(type_feedback)) {
-    return CompareOperationHint::kNone;
-  }
-
-  if (Is<CompareOperationFeedback::kSignedSmall>(type_feedback)) {
-    return CompareOperationHint::kSignedSmall;
-  } else if (Is<CompareOperationFeedback::kNumber>(type_feedback)) {
-    return CompareOperationHint::kNumber;
-  } else if (Is<CompareOperationFeedback::kNumberOrOddball>(type_feedback)) {
-    return CompareOperationHint::kNumberOrOddball;
-  }
-
-  if (Is<CompareOperationFeedback::kInternalizedString>(type_feedback)) {
-    return CompareOperationHint::kInternalizedString;
-  } else if (Is<CompareOperationFeedback::kString>(type_feedback)) {
-    return CompareOperationHint::kString;
-  }
-
-  if (Is<CompareOperationFeedback::kReceiver>(type_feedback)) {
-    return CompareOperationHint::kReceiver;
-  } else if (Is<CompareOperationFeedback::kReceiverOrNullOrUndefined>(
-                 type_feedback)) {
-    return CompareOperationHint::kReceiverOrNullOrUndefined;
-  }
-
-  if (Is<CompareOperationFeedback::kBigInt>(type_feedback)) {
-    return CompareOperationHint::kBigInt;
-  }
-
-  if (Is<CompareOperationFeedback::kSymbol>(type_feedback)) {
-    return CompareOperationHint::kSymbol;
+  switch (type_feedback) {
+    case CompareOperationFeedback::kNone:
+      return CompareOperationHint::kNone;
+    case CompareOperationFeedback::kSignedSmall:
+      return CompareOperationHint::kSignedSmall;
+    case CompareOperationFeedback::kNumber:
+      return CompareOperationHint::kNumber;
+    case CompareOperationFeedback::kNumberOrOddball:
+      return CompareOperationHint::kNumberOrOddball;
+    case CompareOperationFeedback::kInternalizedString:
+      return CompareOperationHint::kInternalizedString;
+    case CompareOperationFeedback::kString:
+      return CompareOperationHint::kString;
+    case CompareOperationFeedback::kSymbol:
+      return CompareOperationHint::kSymbol;
+    case CompareOperationFeedback::kBigInt:
+      return CompareOperationHint::kBigInt;
+    case CompareOperationFeedback::kReceiver:
+      return CompareOperationHint::kReceiver;
+    case CompareOperationFeedback::kReceiverOrNullOrUndefined:
+      return CompareOperationHint::kReceiverOrNullOrUndefined;
+    default:
+      return CompareOperationHint::kAny;
   }
-
-  DCHECK(Is<CompareOperationFeedback::kAny>(type_feedback));
-  return CompareOperationHint::kAny;
+  UNREACHABLE();
 }
 
 // Helper function to transform the feedback to ForInHint.

test/cctest/interpreter/test-interpreter.cc

@@ -2087,6 +2087,7 @@ TEST(InterpreterMixedComparisons) {
                 LoadStringAndAddSpace(&builder, &ast_factory, rhs_cstr,
                                       string_add_slot);
               }
+              break;
             } else {
               CHECK_EQ(which_side, kLhsIsString);
               // Comparison with String on the lhs and HeapNumber on the rhs.
@@ -2120,19 +2121,9 @@ TEST(InterpreterMixedComparisons) {
             if (tester.HasFeedbackMetadata()) {
               MaybeObject feedback = callable.vector().Get(slot);
               CHECK(feedback->IsSmi());
-              if (kComparisonTypes[c] == Token::Value::EQ) {
-                // For sloppy equality, we have more precise feedback.
-                CHECK_EQ(
-                    CompareOperationFeedback::kNumber |
-                        (string_type == kInternalizedStringConstant
-                             ? CompareOperationFeedback::kInternalizedString
-                             : CompareOperationFeedback::kString),
-                    feedback->ToSmi().value());
-              } else {
-                // Comparison with a number and string collects kAny feedback.
-                CHECK_EQ(CompareOperationFeedback::kAny,
-                         feedback->ToSmi().value());
-              }
+              // Comparison with a number and string collects kAny feedback.
+              CHECK_EQ(CompareOperationFeedback::kAny,
+                       feedback->ToSmi().value());
             }
           }
         }

test/mjsunit/regress/regress-5660.js

-// Copyright 2020 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
-
-(function() {
-  function test(a, b) {
-    return a === b;
-  }
-
-  %PrepareFunctionForOptimization(test);
-  assertTrue(test(undefined, undefined));
-  assertTrue(test(undefined, undefined));
-  %OptimizeFunctionOnNextCall(test);
-  assertTrue(test(undefined, undefined));
-})();
-
-(function() {
-  function test(a, b) {
-    return a === b;
-  }
-
-  %PrepareFunctionForOptimization(test);
-  assertTrue(test(true, true));
-  assertTrue(test(true, true));
-  %OptimizeFunctionOnNextCall(test);
-  assertFalse(test(true, 1));
-})();
-
-(function() {
-  function test(a, b) {
-      return a == b;
-  }
-
-  %PrepareFunctionForOptimization(test);
-  assertTrue(test(true, true));
-  assertTrue(test(true, true));
-  %OptimizeFunctionOnNextCall(test);
-  assertTrue(test(true, 1));
-})();