[turbofan] Handle comparison operations in early lowering.

This handles comparison operations (equality and relational) having
number feedback during the early type-hint lowering (i.e. during graph
construction).

R=bmeurer@chromium.org

Change-Id: I97afd6c0d78a790ce38b731f2532ca18d812a32c
Reviewed-on: https://chromium-review.googlesource.com/444766Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43315}

src/compiler/bytecode-graph-builder.cc

@@ -1517,12 +1517,13 @@ Node* BytecodeGraphBuilder::TryBuildSimplifiedBinaryOp(const Operator* op,
   Node* effect = environment()->GetEffectDependency();
   Node* control = environment()->GetControlDependency();
   JSTypeHintLowering type_hint_lowering(jsgraph(), feedback_vector());
-  Reduction early_reduction = type_hint_lowering.ReduceBinaryOperation(
+  EarlyReduction early_reduction = type_hint_lowering.ReduceBinaryOperation(
       op, left, right, effect, control, slot);
-  if (early_reduction.Changed()) {
-    Node* node = early_reduction.replacement();
-    if (node->op()->EffectOutputCount() > 0) {
-      environment()->UpdateEffectDependency(node);
+  if (early_reduction.has_reduction()) {
+    Node* node = early_reduction.value();
+    if (early_reduction.has_effect()) {
+      DCHECK_GT(early_reduction.effect()->op()->EffectOutputCount(), 0);
+      environment()->UpdateEffectDependency(early_reduction.effect());
     }
     return node;
   }
@@ -1745,12 +1746,25 @@ void BytecodeGraphBuilder::VisitGetSuperConstructor() {
                               Environment::kAttachFrameState);
 }
 
-void BytecodeGraphBuilder::BuildCompareOp(const Operator* js_op) {
+void BytecodeGraphBuilder::BuildCompareOp(const Operator* op) {
   PrepareEagerCheckpoint();
   Node* left =
       environment()->LookupRegister(bytecode_iterator().GetRegisterOperand(0));
   Node* right = environment()->LookupAccumulator();
-  Node* node = NewNode(js_op, left, right);
+
+  Node* node = nullptr;
+  int slot_index = bytecode_iterator().GetIndexOperand(1);
+  if (slot_index != 0) {
+    FeedbackSlot slot = feedback_vector()->ToSlot(slot_index);
+    if (Node* simplified = TryBuildSimplifiedBinaryOp(op, left, right, slot)) {
+      node = simplified;
+    } else {
+      node = NewNode(op, left, right);
+    }
+  } else {
+    node = NewNode(op, left, right);
+  }
+
   environment()->BindAccumulator(node, Environment::kAttachFrameState);
 }
 
@@ -1782,12 +1796,21 @@ void BytecodeGraphBuilder::VisitTestGreaterThanOrEqual() {
   BuildCompareOp(javascript()->GreaterThanOrEqual(GetCompareOperationHint()));
 }
 
+void BytecodeGraphBuilder::BuildTestingOp(const Operator* op) {
+  PrepareEagerCheckpoint();
+  Node* left =
+      environment()->LookupRegister(bytecode_iterator().GetRegisterOperand(0));
+  Node* right = environment()->LookupAccumulator();
+  Node* node = NewNode(op, left, right);
+  environment()->BindAccumulator(node, Environment::kAttachFrameState);
+}
+
 void BytecodeGraphBuilder::VisitTestIn() {
-  BuildCompareOp(javascript()->HasProperty());
+  BuildTestingOp(javascript()->HasProperty());
 }
 
 void BytecodeGraphBuilder::VisitTestInstanceOf() {
-  BuildCompareOp(javascript()->InstanceOf());
+  BuildTestingOp(javascript()->InstanceOf());
 }
 
 void BytecodeGraphBuilder::VisitTestUndetectable() {

src/compiler/bytecode-graph-builder.h

@@ -155,6 +155,7 @@ class BytecodeGraphBuilder {
   void BuildBinaryOp(const Operator* op);
   void BuildBinaryOpWithImmediate(const Operator* op);
   void BuildCompareOp(const Operator* op);
+  void BuildTestingOp(const Operator* op);
   void BuildDelete(LanguageMode language_mode);
   void BuildCastOperator(const Operator* op);
   void BuildForInPrepare();

src/compiler/js-type-hint-lowering.cc

@@ -33,6 +33,12 @@ class JSSpeculativeBinopBuilder final {
     return nexus.GetBinaryOperationFeedback();
   }
 
+  CompareOperationHint GetCompareOperationHint() {
+    DCHECK_EQ(FeedbackSlotKind::kCompareOp, feedback_vector()->GetKind(slot_));
+    CompareICNexus nexus(feedback_vector(), slot_);
+    return nexus.GetCompareOperationFeedback();
+  }
+
   bool GetBinaryNumberOperationHint(NumberOperationHint* hint) {
     switch (GetBinaryOperationHint()) {
       case BinaryOperationHint::kSignedSmall:
@@ -52,6 +58,27 @@ class JSSpeculativeBinopBuilder final {
     return false;
   }
 
+  bool GetCompareNumberOperationHint(NumberOperationHint* hint) {
+    switch (GetCompareOperationHint()) {
+      case CompareOperationHint::kSignedSmall:
+        *hint = NumberOperationHint::kSignedSmall;
+        return true;
+      case CompareOperationHint::kNumber:
+        *hint = NumberOperationHint::kNumber;
+        return true;
+      case CompareOperationHint::kNumberOrOddball:
+        *hint = NumberOperationHint::kNumberOrOddball;
+        return true;
+      case CompareOperationHint::kAny:
+      case CompareOperationHint::kNone:
+      case CompareOperationHint::kString:
+      case CompareOperationHint::kReceiver:
+      case CompareOperationHint::kInternalizedString:
+        break;
+    }
+    return false;
+  }
+
   const Operator* SpeculativeNumberOp(NumberOperationHint hint) {
     switch (op_->opcode()) {
       case IrOpcode::kJSAdd:
@@ -83,7 +110,26 @@ class JSSpeculativeBinopBuilder final {
     return nullptr;
   }
 
-  Node* BuildSpeculativeOperator(const Operator* op) {
+  const Operator* SpeculativeCompareOp(NumberOperationHint hint) {
+    switch (op_->opcode()) {
+      case IrOpcode::kJSLessThan:
+        return simplified()->SpeculativeNumberLessThan(hint);
+      case IrOpcode::kJSGreaterThan:
+        std::swap(left_, right_);  // a > b => b < a
+        return simplified()->SpeculativeNumberLessThan(hint);
+      case IrOpcode::kJSLessThanOrEqual:
+        return simplified()->SpeculativeNumberLessThanOrEqual(hint);
+      case IrOpcode::kJSGreaterThanOrEqual:
+        std::swap(left_, right_);  // a >= b => b <= a
+        return simplified()->SpeculativeNumberLessThanOrEqual(hint);
+      default:
+        break;
+    }
+    UNREACHABLE();
+    return nullptr;
+  }
+
+  Node* BuildSpeculativeOperation(const Operator* op) {
     DCHECK_EQ(2, op->ValueInputCount());
     DCHECK_EQ(1, op->EffectInputCount());
     DCHECK_EQ(1, op->ControlInputCount());
@@ -91,9 +137,46 @@ class JSSpeculativeBinopBuilder final {
     DCHECK_EQ(false, OperatorProperties::HasContextInput(op));
     DCHECK_EQ(1, op->EffectOutputCount());
     DCHECK_EQ(0, op->ControlOutputCount());
-    return graph()->NewNode(op, left_, right_, effect_, control_);
+    Node* node = graph()->NewNode(op, left_, right_, effect_, control_);
+    effect_ = node;  // Update the effect dependency.
+    return node;
+  }
+
+  Node* BuildInvert(Node* input) {
+    return graph()->NewNode(simplified()->BooleanNot(), input);
+  }
+
+  Node* TryBuildNumberBinop() {
+    NumberOperationHint hint;
+    if (GetBinaryNumberOperationHint(&hint)) {
+      const Operator* op = SpeculativeNumberOp(hint);
+      Node* node = BuildSpeculativeOperation(op);
+      return node;
+    }
+    return nullptr;
+  }
+
+  Node* TryBuildNumberEqual(bool invert) {
+    NumberOperationHint hint;
+    if (GetCompareNumberOperationHint(&hint)) {
+      const Operator* op = simplified()->SpeculativeNumberEqual(hint);
+      Node* compare = BuildSpeculativeOperation(op);
+      return invert ? BuildInvert(compare) : compare;
+    }
+    return nullptr;
+  }
+
+  Node* TryBuildNumberCompare() {
+    NumberOperationHint hint;
+    if (GetCompareNumberOperationHint(&hint)) {
+      const Operator* op = SpeculativeCompareOp(hint);
+      Node* node = BuildSpeculativeOperation(op);
+      return node;
+    }
+    return nullptr;
   }
 
+  Node* effect() const { return effect_; }
   JSGraph* jsgraph() const { return lowering_->jsgraph(); }
   Graph* graph() const { return jsgraph()->graph(); }
   JSOperatorBuilder* javascript() { return jsgraph()->javascript(); }
@@ -117,11 +200,36 @@ JSTypeHintLowering::JSTypeHintLowering(JSGraph* jsgraph,
                                        Handle<FeedbackVector> feedback_vector)
     : jsgraph_(jsgraph), feedback_vector_(feedback_vector) {}
 
-Reduction JSTypeHintLowering::ReduceBinaryOperation(const Operator* op,
-                                                    Node* left, Node* right,
-                                                    Node* effect, Node* control,
-                                                    FeedbackSlot slot) {
+EarlyReduction JSTypeHintLowering::ReduceBinaryOperation(
+    const Operator* op, Node* left, Node* right, Node* effect, Node* control,
+    FeedbackSlot slot) {
   switch (op->opcode()) {
+    case IrOpcode::kJSEqual:
+    case IrOpcode::kJSStrictEqual: {
+      JSSpeculativeBinopBuilder b(this, op, left, right, effect, control, slot);
+      if (Node* node = b.TryBuildNumberEqual(false)) {
+        return EarlyReduction(node, b.effect());
+      }
+      break;
+    }
+    case IrOpcode::kJSNotEqual:
+    case IrOpcode::kJSStrictNotEqual: {
+      JSSpeculativeBinopBuilder b(this, op, left, right, effect, control, slot);
+      if (Node* node = b.TryBuildNumberEqual(true)) {
+        return EarlyReduction(node, b.effect());
+      }
+      break;
+    }
+    case IrOpcode::kJSLessThan:
+    case IrOpcode::kJSGreaterThan:
+    case IrOpcode::kJSLessThanOrEqual:
+    case IrOpcode::kJSGreaterThanOrEqual: {
+      JSSpeculativeBinopBuilder b(this, op, left, right, effect, control, slot);
+      if (Node* node = b.TryBuildNumberCompare()) {
+        return EarlyReduction(node, b.effect());
+      }
+      break;
+    }
     case IrOpcode::kJSBitwiseOr:
     case IrOpcode::kJSBitwiseXor:
     case IrOpcode::kJSBitwiseAnd:
@@ -134,10 +242,8 @@ Reduction JSTypeHintLowering::ReduceBinaryOperation(const Operator* op,
     case IrOpcode::kJSDivide:
     case IrOpcode::kJSModulus: {
       JSSpeculativeBinopBuilder b(this, op, left, right, effect, control, slot);
-      NumberOperationHint hint;
-      if (b.GetBinaryNumberOperationHint(&hint)) {
-        Node* node = b.BuildSpeculativeOperator(b.SpeculativeNumberOp(hint));
-        return Reduction(node);
+      if (Node* node = b.TryBuildNumberBinop()) {
+        return EarlyReduction(node, b.effect());
       }
       break;
     }
@@ -145,7 +251,7 @@ Reduction JSTypeHintLowering::ReduceBinaryOperation(const Operator* op,
       UNREACHABLE();
       break;
   }
-  return Reduction();
+  return EarlyReduction();
 }
 
 }  // namespace compiler

src/compiler/js-type-hint-lowering.h

@@ -5,14 +5,20 @@
 #ifndef V8_COMPILER_JS_TYPE_HINT_LOWERING_H_
 #define V8_COMPILER_JS_TYPE_HINT_LOWERING_H_
 
-#include "src/compiler/graph-reducer.h"
 #include "src/handles.h"
 
 namespace v8 {
 namespace internal {
+
+// Forward declarations.
+class FeedbackSlot;
+
 namespace compiler {
 
 // Forward declarations.
+class EarlyReduction;
+class Node;
+class Operator;
 class JSGraph;
 
 // The type-hint lowering consumes feedback about data operations (i.e. unary
@@ -28,10 +34,11 @@ class JSTypeHintLowering {
  public:
   JSTypeHintLowering(JSGraph* jsgraph, Handle<FeedbackVector> feedback_vector);
 
-  // Potential reduction of binary (arithmetic, logical and shift) operations.
-  Reduction ReduceBinaryOperation(const Operator* op, Node* left, Node* right,
-                                  Node* effect, Node* control,
-                                  FeedbackSlot slot);
+  // Potential reduction of binary (arithmetic, logical, shift, equalities and
+  // relational comparison) operations.
+  EarlyReduction ReduceBinaryOperation(const Operator*, Node* left, Node* right,
+                                       Node* effect, Node* control,
+                                       FeedbackSlot slot);
 
  private:
   friend class JSSpeculativeBinopBuilder;
@@ -47,6 +54,25 @@ class JSTypeHintLowering {
   DISALLOW_COPY_AND_ASSIGN(JSTypeHintLowering);
 };
 
+// The result of a successful early reduction is a {value} node and an optional
+// {effect} node (which might be different from the value). In case reduction
+// failed, none of the above nodes are provided.
+class EarlyReduction final {
+ public:
+  EarlyReduction() : value_(nullptr), effect_(nullptr) {}
+  EarlyReduction(Node* value, Node* effect) : value_(value), effect_(effect) {}
+
+  Node* value() const { return value_; }
+  Node* effect() const { return effect_; }
+
+  bool has_reduction() const { return value_ != nullptr; }
+  bool has_effect() const { return effect_ != nullptr; }
+
+ private:
+  Node* value_;
+  Node* effect_;
+};
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

src/compiler/js-typed-lowering.cc

@@ -30,30 +30,6 @@ class JSBinopReduction final {
   JSBinopReduction(JSTypedLowering* lowering, Node* node)
       : lowering_(lowering), node_(node) {}
 
-  bool GetCompareNumberOperationHint(NumberOperationHint* hint) {
-    if (lowering_->flags() & JSTypedLowering::kDeoptimizationEnabled) {
-      DCHECK_EQ(1, node_->op()->EffectOutputCount());
-      switch (CompareOperationHintOf(node_->op())) {
-        case CompareOperationHint::kSignedSmall:
-          *hint = NumberOperationHint::kSignedSmall;
-          return true;
-        case CompareOperationHint::kNumber:
-          *hint = NumberOperationHint::kNumber;
-          return true;
-        case CompareOperationHint::kNumberOrOddball:
-          *hint = NumberOperationHint::kNumberOrOddball;
-          return true;
-        case CompareOperationHint::kAny:
-        case CompareOperationHint::kNone:
-        case CompareOperationHint::kString:
-        case CompareOperationHint::kReceiver:
-        case CompareOperationHint::kInternalizedString:
-          break;
-      }
-    }
-    return false;
-  }
-
   bool IsInternalizedStringCompareOperation() {
     if (lowering_->flags() & JSTypedLowering::kDeoptimizationEnabled) {
       DCHECK_EQ(1, node_->op()->EffectOutputCount());
@@ -253,69 +229,10 @@ class JSBinopReduction final {
     return lowering_->Changed(node_);
   }
 
-  Reduction ChangeToSpeculativeOperator(const Operator* op, bool invert,
-                                        Type* upper_bound) {
-    DCHECK_EQ(1, op->EffectInputCount());
-    DCHECK_EQ(1, op->EffectOutputCount());
-    DCHECK_EQ(false, OperatorProperties::HasContextInput(op));
-    DCHECK_EQ(1, op->ControlInputCount());
-    DCHECK_EQ(0, op->ControlOutputCount());
-    DCHECK_EQ(0, OperatorProperties::GetFrameStateInputCount(op));
-    DCHECK_EQ(2, op->ValueInputCount());
-
-    DCHECK_EQ(1, node_->op()->EffectInputCount());
-    DCHECK_EQ(1, node_->op()->EffectOutputCount());
-    DCHECK_EQ(1, node_->op()->ControlInputCount());
-    DCHECK_EQ(2, node_->op()->ValueInputCount());
-
-    // Reconnect the control output to bypass the IfSuccess node and
-    // possibly disconnect from the IfException node.
-    for (Edge edge : node_->use_edges()) {
-      Node* const user = edge.from();
-      DCHECK(!user->IsDead());
-      if (NodeProperties::IsControlEdge(edge)) {
-        if (user->opcode() == IrOpcode::kIfSuccess) {
-          user->ReplaceUses(NodeProperties::GetControlInput(node_));
-          user->Kill();
-        } else {
-          DCHECK_EQ(user->opcode(), IrOpcode::kIfException);
-          edge.UpdateTo(jsgraph()->Dead());
-        }
-      }
-    }
-
-    // Remove the frame state and the context.
-    if (OperatorProperties::HasFrameStateInput(node_->op())) {
-      node_->RemoveInput(NodeProperties::FirstFrameStateIndex(node_));
-    }
-    node_->RemoveInput(NodeProperties::FirstContextIndex(node_));
-
-    NodeProperties::ChangeOp(node_, op);
-
-    // Update the type to number.
-    Type* node_type = NodeProperties::GetType(node_);
-    NodeProperties::SetType(node_,
-                            Type::Intersect(node_type, upper_bound, zone()));
-
-    if (invert) {
-      // Insert an boolean not to invert the value.
-      Node* value = graph()->NewNode(simplified()->BooleanNot(), node_);
-      node_->ReplaceUses(value);
-      // Note: ReplaceUses() smashes all uses, so smash it back here.
-      value->ReplaceInput(0, node_);
-      return lowering_->Replace(value);
-    }
-    return lowering_->Changed(node_);
-  }
-
   Reduction ChangeToPureOperator(const Operator* op, Type* type) {
     return ChangeToPureOperator(op, false, type);
   }
 
-  Reduction ChangeToSpeculativeOperator(const Operator* op, Type* type) {
-    return ChangeToSpeculativeOperator(op, false, type);
-  }
-
   const Operator* NumberOp() {
     switch (node_->opcode()) {
       case IrOpcode::kJSAdd:
@@ -349,6 +266,10 @@ class JSBinopReduction final {
 
   const Operator* NumberOpFromSpeculativeNumberOp() {
     switch (node_->opcode()) {
+      case IrOpcode::kSpeculativeNumberLessThan:
+        return simplified()->NumberLessThan();
+      case IrOpcode::kSpeculativeNumberLessThanOrEqual:
+        return simplified()->NumberLessThanOrEqual();
       case IrOpcode::kSpeculativeNumberAdd:
         return simplified()->NumberAdd();
       case IrOpcode::kSpeculativeNumberSubtract:
@@ -770,6 +691,15 @@ Reduction JSTypedLowering::ReduceCreateConsString(Node* node) {
   return Changed(node);
 }
 
+Reduction JSTypedLowering::ReduceSpeculativeNumberComparison(Node* node) {
+  JSBinopReduction r(this, node);
+  if (r.BothInputsAre(Type::Signed32()) ||
+      r.BothInputsAre(Type::Unsigned32())) {
+    return r.ChangeToPureOperator(r.NumberOpFromSpeculativeNumberOp());
+  }
+  return Changed(node);
+}
+
 Reduction JSTypedLowering::ReduceJSComparison(Node* node) {
   JSBinopReduction r(this, node);
   if (r.BothInputsAre(Type::String())) {
@@ -797,16 +727,12 @@ Reduction JSTypedLowering::ReduceJSComparison(Node* node) {
     return Changed(node);
   }
 
-  NumberOperationHint hint;
   const Operator* less_than;
   const Operator* less_than_or_equal;
   if (r.BothInputsAre(Type::Signed32()) ||
       r.BothInputsAre(Type::Unsigned32())) {
     less_than = simplified()->NumberLessThan();
     less_than_or_equal = simplified()->NumberLessThanOrEqual();
-  } else if (r.GetCompareNumberOperationHint(&hint)) {
-    less_than = simplified()->SpeculativeNumberLessThan(hint);
-    less_than_or_equal = simplified()->SpeculativeNumberLessThanOrEqual(hint);
   } else if (r.OneInputCannotBe(Type::StringOrReceiver()) &&
              (r.BothInputsAre(Type::PlainPrimitive()) ||
               !(flags() & kDeoptimizationEnabled))) {
@@ -839,11 +765,19 @@ Reduction JSTypedLowering::ReduceJSComparison(Node* node) {
     default:
       return NoChange();
   }
-  if (comparison->EffectInputCount() > 0) {
-    return r.ChangeToSpeculativeOperator(comparison, Type::Boolean());
-  } else {
-    return r.ChangeToPureOperator(comparison);
+  return r.ChangeToPureOperator(comparison);
+}
+
+Reduction JSTypedLowering::ReduceSpeculativeNumberEqual(Node* node) {
+  JSBinopReduction r(this, node);
+  if (r.BothInputsAre(Type::Boolean())) {
+    return r.ChangeToPureOperator(simplified()->ReferenceEqual());
+  }
+  if (r.BothInputsAre(Type::Signed32()) ||
+      r.BothInputsAre(Type::Unsigned32())) {
+    return r.ChangeToPureOperator(simplified()->NumberEqual());
   }
+  return NoChange();
 }
 
 Reduction JSTypedLowering::ReduceJSTypeOf(Node* node) {
@@ -964,13 +898,9 @@ Reduction JSTypedLowering::ReduceJSEqual(Node* node, bool invert) {
     return Changed(node);
   }
 
-  NumberOperationHint hint;
   if (r.BothInputsAre(Type::Signed32()) ||
       r.BothInputsAre(Type::Unsigned32())) {
     return r.ChangeToPureOperator(simplified()->NumberEqual(), invert);
-  } else if (r.GetCompareNumberOperationHint(&hint)) {
-    return r.ChangeToSpeculativeOperator(
-        simplified()->SpeculativeNumberEqual(hint), invert, Type::Boolean());
   } else if (r.BothInputsAre(Type::Number())) {
     return r.ChangeToPureOperator(simplified()->NumberEqual(), invert);
   } else if (r.IsReceiverCompareOperation()) {
@@ -1021,13 +951,9 @@ Reduction JSTypedLowering::ReduceJSStrictEqual(Node* node, bool invert) {
     return r.ChangeToPureOperator(simplified()->StringEqual(), invert);
   }
 
-  NumberOperationHint hint;
   if (r.BothInputsAre(Type::Signed32()) ||
       r.BothInputsAre(Type::Unsigned32())) {
     return r.ChangeToPureOperator(simplified()->NumberEqual(), invert);
-  } else if (r.GetCompareNumberOperationHint(&hint)) {
-    return r.ChangeToSpeculativeOperator(
-        simplified()->SpeculativeNumberEqual(hint), invert, Type::Boolean());
   } else if (r.BothInputsAre(Type::Number())) {
     return r.ChangeToPureOperator(simplified()->NumberEqual(), invert);
   } else if (r.IsReceiverCompareOperation()) {
@@ -2451,6 +2377,11 @@ Reduction JSTypedLowering::Reduce(Node* node) {
     case IrOpcode::kSpeculativeNumberDivide:
     case IrOpcode::kSpeculativeNumberModulus:
       return ReduceSpeculativeNumberBinop(node);
+    case IrOpcode::kSpeculativeNumberEqual:
+      return ReduceSpeculativeNumberEqual(node);
+    case IrOpcode::kSpeculativeNumberLessThan:
+    case IrOpcode::kSpeculativeNumberLessThanOrEqual:
+      return ReduceSpeculativeNumberComparison(node);
     default:
       break;
   }

src/compiler/js-typed-lowering.h

@@ -86,6 +86,8 @@ class V8_EXPORT_PRIVATE JSTypedLowering final
   Reduction ReduceCreateConsString(Node* node);
   Reduction ReduceSpeculativeNumberAdd(Node* node);
   Reduction ReduceSpeculativeNumberBinop(Node* node);
+  Reduction ReduceSpeculativeNumberEqual(Node* node);
+  Reduction ReduceSpeculativeNumberComparison(Node* node);
 
   Factory* factory() const;
   Graph* graph() const;