[turbofan] Fix abstract equality with undetectable

The code generated by TurboFan was incorrect when comparing to
non-oddball undetectables using abstract equality. In particular,
%GetUndetectable() == %GetUndetectable() did not return false.

Bug: chromium:1051008
Change-Id: Ib62adc72a20aa6cca9ef6499d5fe7429f04623cf
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2187498
Commit-Queue: Nico Hartmann <nicohartmann@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67647}

src/compiler/graph-assembler.cc

@@ -659,6 +659,11 @@ TNode<Boolean> JSGraphAssembler::ObjectIsCallable(TNode<Object> value) {
       graph()->NewNode(simplified()->ObjectIsCallable(), value));
 }
 
+TNode<Boolean> JSGraphAssembler::ObjectIsUndetectable(TNode<Object> value) {
+  return AddNode<Boolean>(
+      graph()->NewNode(simplified()->ObjectIsUndetectable(), value));
+}
+
 Node* JSGraphAssembler::CheckIf(Node* cond, DeoptimizeReason reason) {
   return AddNode(graph()->NewNode(simplified()->CheckIf(reason), cond, effect(),
                                   control()));

src/compiler/graph-assembler.h

@@ -825,6 +825,7 @@ class V8_EXPORT_PRIVATE JSGraphAssembler : public GraphAssembler {
   TNode<String> StringSubstring(TNode<String> string, TNode<Number> from,
                                 TNode<Number> to);
   TNode<Boolean> ObjectIsCallable(TNode<Object> value);
+  TNode<Boolean> ObjectIsUndetectable(TNode<Object> value);
   Node* CheckIf(Node* cond, DeoptimizeReason reason);
   TNode<Boolean> NumberIsFloat64Hole(TNode<Number> value);
   TNode<Boolean> ToBoolean(TNode<Object> value);

src/compiler/js-typed-lowering.cc

@@ -9,6 +9,7 @@
 #include "src/codegen/code-factory.h"
 #include "src/compiler/access-builder.h"
 #include "src/compiler/allocation-builder.h"
+#include "src/compiler/graph-assembler.h"
 #include "src/compiler/js-graph.h"
 #include "src/compiler/js-heap-broker.h"
 #include "src/compiler/linkage.h"
@@ -778,9 +779,9 @@ Reduction JSTypedLowering::ReduceJSEqual(Node* node) {
   if (r.BothInputsAre(Type::Receiver())) {
     return r.ChangeToPureOperator(simplified()->ReferenceEqual());
   }
-  if (r.OneInputIs(Type::Undetectable())) {
+  if (r.OneInputIs(Type::NullOrUndefined())) {
     RelaxEffectsAndControls(node);
-    node->RemoveInput(r.LeftInputIs(Type::Undetectable()) ? 0 : 1);
+    node->RemoveInput(r.LeftInputIs(Type::NullOrUndefined()) ? 0 : 1);
     node->TrimInputCount(1);
     NodeProperties::ChangeOp(node, simplified()->ObjectIsUndetectable());
     return Changed(node);
@@ -808,32 +809,40 @@ Reduction JSTypedLowering::ReduceJSEqual(Node* node) {
     // Known that both sides are Receiver, Null or Undefined, the
     // abstract equality operation can be performed like this:
     //
-    //   if ObjectIsUndetectable(left)
-    //     then ObjectIsUndetectable(right)
-    //     else ReferenceEqual(left, right)
-    //
-    Node* left = r.left();
-    Node* right = r.right();
-    Node* effect = r.effect();
-    Node* control = r.control();
-
-    Node* check = graph()->NewNode(simplified()->ObjectIsUndetectable(), left);
-    Node* branch =
-        graph()->NewNode(common()->Branch(BranchHint::kFalse), check, control);
-
-    Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
-    Node* vtrue = graph()->NewNode(simplified()->ObjectIsUndetectable(), right);
-
-    Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
-    Node* vfalse =
-        graph()->NewNode(simplified()->ReferenceEqual(), left, right);
-
-    control = graph()->NewNode(common()->Merge(2), if_true, if_false);
-    Node* value =
-        graph()->NewNode(common()->Phi(MachineRepresentation::kTagged, 2),
-                         vtrue, vfalse, control);
-    ReplaceWithValue(node, value, effect, control);
+    // if left == undefined || left == null
+    //    then ObjectIsUndetectable(right)
+    // else if right == undefined || right == null
+    //    then ObjectIsUndetectable(left)
+    // else ReferenceEqual(left, right)
+#define __ gasm.
+    JSGraphAssembler gasm(jsgraph(), jsgraph()->zone());
+    gasm.InitializeEffectControl(r.effect(), r.control());
+
+    auto lhs = TNode<Object>::UncheckedCast(r.left());
+    auto rhs = TNode<Object>::UncheckedCast(r.right());
+
+    auto done = __ MakeLabel(MachineRepresentation::kTagged);
+    auto check_undetectable = __ MakeLabel(MachineRepresentation::kTagged);
+
+    __ GotoIf(__ ReferenceEqual(lhs, __ UndefinedConstant()),
+              &check_undetectable, rhs);
+    __ GotoIf(__ ReferenceEqual(lhs, __ NullConstant()), &check_undetectable,
+              rhs);
+    __ GotoIf(__ ReferenceEqual(rhs, __ UndefinedConstant()),
+              &check_undetectable, lhs);
+    __ GotoIf(__ ReferenceEqual(rhs, __ NullConstant()), &check_undetectable,
+              lhs);
+    __ Goto(&done, __ ReferenceEqual(lhs, rhs));
+
+    __ Bind(&check_undetectable);
+    __ Goto(&done,
+            __ ObjectIsUndetectable(check_undetectable.PhiAt<Object>(0)));
+
+    __ Bind(&done);
+    Node* value = done.PhiAt(0);
+    ReplaceWithValue(node, value, gasm.effect(), gasm.control());
     return Replace(value);
+#undef __
   } else if (r.IsStringCompareOperation()) {
     r.CheckInputsToString();
     return r.ChangeToPureOperator(simplified()->StringEqual());

test/mjsunit/compiler/abstract-equal-undetectable.js

@@ -91,10 +91,6 @@ const undetectable = %GetUndetectable();
   %OptimizeFunctionOnNextCall(foo);
   assertTrue(foo(b));
   assertFalse(foo(a));
-
-  // TurboFan doesn't need to bake in feedback, since it sees the undetectable.
-  assertFalse(foo(1));
-  assertOptimized(foo);
 })();
 
 // Unknown undetectable on one side strict equality with receiver.
@@ -124,3 +120,30 @@ const undetectable = %GetUndetectable();
   assertFalse(foo(1));
   assertUnoptimized(foo);
 })();
+
+// Unknown undetectable on both sides.
+(function() {
+  const a = undetectable;
+
+  function foo(a, b) { return a == b; }
+
+  %PrepareFunctionForOptimization(foo);
+  assertTrue(foo(a, a));
+  assertTrue(foo(a, undefined));
+  assertTrue(foo(undefined, a));
+  assertFalse(foo(a, %GetUndetectable()));
+  assertFalse(foo(%GetUndetectable(), a));
+  assertFalse(foo(%GetUndetectable(), %GetUndetectable()));
+  %OptimizeFunctionOnNextCall(foo);
+  assertTrue(foo(a, a));
+  assertTrue(foo(a, undefined));
+  assertTrue(foo(undefined, a));
+  assertFalse(foo(a, %GetUndetectable()));
+  assertFalse(foo(%GetUndetectable(), a));
+  assertFalse(foo(%GetUndetectable(), %GetUndetectable()));
+  assertOptimized(foo);
+
+  // TurboFan bakes in feedback on the inputs.
+  assertFalse(foo(1));
+  assertUnoptimized(foo);
+})();