[turbofan] Adds speculative opcodes for shift right.

Drive-by fix: actually match the hint in the IsSpeculativeBinopMatcher.

Review-Url: https://codereview.chromium.org/2191883002
Cr-Commit-Position: refs/heads/master@{#38176}

src/compiler/js-typed-lowering.cc

@@ -590,38 +590,38 @@ Reduction JSTypedLowering::ReduceInt32Binop(Node* node, const Operator* intOp) {
   return r.ChangeToPureOperator(intOp, Type::Integral32());
 }
 
-Reduction JSTypedLowering::ReduceShiftLeft(Node* node) {
+Reduction JSTypedLowering::ReduceUI32Shift(Node* node,
+                                           Signedness left_signedness,
+                                           const Operator* shift_op) {
   if (flags() & kDisableIntegerBinaryOpReduction) return NoChange();
-
   JSBinopReduction r(this, node);
   BinaryOperationHints::Hint feedback = r.GetNumberBinaryOperationFeedback();
   if (feedback != BinaryOperationHints::kAny) {
+    Operator const* speculative_op;
+    if (shift_op->opcode() == IrOpcode::kNumberShiftLeft) {
+      speculative_op = simplified()->SpeculativeNumberShiftLeft(feedback);
+    } else if (shift_op->opcode() == IrOpcode::kNumberShiftRightLogical) {
+      speculative_op =
+          simplified()->SpeculativeNumberShiftRightLogical(feedback);
+    } else {
+      DCHECK(shift_op->opcode() == IrOpcode::kNumberShiftRight);
+      speculative_op = simplified()->SpeculativeNumberShiftRight(feedback);
+    }
     return r.ChangeToSpeculativeOperator(
-        simplified()->SpeculativeNumberShiftLeft(feedback), Type::Signed32());
+        speculative_op, shift_op->opcode() == IrOpcode::kNumberShiftRightLogical
+                            ? Type::Unsigned32()
+                            : Type::Signed32());
   }
 
   // If deoptimization is enabled we rely on type feedback.
   if (r.BothInputsAre(Type::PlainPrimitive()) ||
       !(flags() & kDeoptimizationEnabled)) {
     r.ConvertInputsToNumber();
-    r.ConvertInputsToUI32(kSigned, kUnsigned);
-    return r.ChangeToPureOperator(simplified()->NumberShiftLeft(),
-                                  Type::Number());
-  }
-  return NoChange();
-}
-
-Reduction JSTypedLowering::ReduceUI32Shift(Node* node,
-                                           Signedness left_signedness,
-                                           const Operator* shift_op) {
-  if (flags() & kDisableIntegerBinaryOpReduction) return NoChange();
-
-  JSBinopReduction r(this, node);
-  r.ConvertInputsToNumber();
     r.ConvertInputsToUI32(left_signedness, kUnsigned);
     return r.ChangeToPureOperator(shift_op);
   }
-
+  return NoChange();
+}
 
 Reduction JSTypedLowering::ReduceJSComparison(Node* node) {
   JSBinopReduction r(this, node);
@@ -2049,7 +2049,7 @@ Reduction JSTypedLowering::Reduce(Node* node) {
     case IrOpcode::kJSBitwiseAnd:
       return ReduceInt32Binop(node, simplified()->NumberBitwiseAnd());
     case IrOpcode::kJSShiftLeft:
-      return ReduceShiftLeft(node);
+      return ReduceUI32Shift(node, kSigned, simplified()->NumberShiftLeft());
     case IrOpcode::kJSShiftRight:
       return ReduceUI32Shift(node, kSigned, simplified()->NumberShiftRight());
     case IrOpcode::kJSShiftRightLogical:

src/compiler/js-typed-lowering.h

@@ -86,7 +86,6 @@ class JSTypedLowering final : public AdvancedReducer {
   Reduction ReduceJSSubtract(Node* node);
   Reduction ReduceJSDivide(Node* node);
   Reduction ReduceInt32Binop(Node* node, const Operator* intOp);
-  Reduction ReduceShiftLeft(Node* node);
   Reduction ReduceUI32Shift(Node* node, Signedness left_signedness,
                             const Operator* shift_op);
 

src/compiler/opcodes.h

@@ -227,6 +227,8 @@
   V(NumberBitwiseAnd)                   \
   V(NumberShiftLeft)                    \
   V(SpeculativeNumberShiftLeft)         \
+  V(SpeculativeNumberShiftRight)        \
+  V(SpeculativeNumberShiftRightLogical) \
   V(NumberShiftRight)                   \
   V(NumberShiftRightLogical)            \
   V(NumberImul)                         \

src/compiler/simplified-lowering.cc

@@ -1692,6 +1692,35 @@ class RepresentationSelector {
         }
         return;
       }
+      case IrOpcode::kSpeculativeNumberShiftRight: {
+        // ToNumber(x) can throw if x is either a Receiver or a Symbol, so we
+        // can only eliminate an unused speculative number operation if we know
+        // that the inputs are PlainPrimitive, which excludes everything that's
+        // might have side effects or throws during a ToNumber conversion.
+        if (BothInputsAre(node, Type::PlainPrimitive())) {
+          if (truncation.IsUnused()) return VisitUnused(node);
+        }
+        if (BothInputsAre(node, Type::NumberOrOddball())) {
+          Type* rhs_type = GetUpperBound(node->InputAt(1));
+          VisitBinop(node, UseInfo::TruncatingWord32(),
+                     UseInfo::TruncatingWord32(),
+                     MachineRepresentation::kWord32);
+          if (lower()) {
+            lowering->DoShift(node, lowering->machine()->Word32Sar(), rhs_type);
+          }
+          return;
+        }
+        BinaryOperationHints::Hint hint = BinaryOperationHintOf(node->op());
+        Type* rhs_type = GetUpperBound(node->InputAt(1));
+        VisitBinop(node, hint == BinaryOperationHints::kNumberOrOddball
+                             ? UseInfo::CheckedNumberOrOddballAsWord32()
+                             : UseInfo::CheckedSigned32AsWord32(),
+                   MachineRepresentation::kWord32, Type::Signed32());
+        if (lower()) {
+          lowering->DoShift(node, lowering->machine()->Word32Sar(), rhs_type);
+        }
+        return;
+      }
       case IrOpcode::kNumberShiftRightLogical: {
         Type* rhs_type = GetUpperBound(node->InputAt(1));
         VisitBinop(node, UseInfo::TruncatingWord32(),
@@ -1701,6 +1730,35 @@ class RepresentationSelector {
         }
         return;
       }
+      case IrOpcode::kSpeculativeNumberShiftRightLogical: {
+        // ToNumber(x) can throw if x is either a Receiver or a Symbol, so we
+        // can only eliminate an unused speculative number operation if we know
+        // that the inputs are PlainPrimitive, which excludes everything that's
+        // might have side effects or throws during a ToNumber conversion.
+        if (BothInputsAre(node, Type::PlainPrimitive())) {
+          if (truncation.IsUnused()) return VisitUnused(node);
+        }
+        if (BothInputsAre(node, Type::NumberOrOddball())) {
+          Type* rhs_type = GetUpperBound(node->InputAt(1));
+          VisitBinop(node, UseInfo::TruncatingWord32(),
+                     UseInfo::TruncatingWord32(),
+                     MachineRepresentation::kWord32);
+          if (lower()) {
+            lowering->DoShift(node, lowering->machine()->Word32Shr(), rhs_type);
+          }
+          return;
+        }
+        BinaryOperationHints::Hint hint = BinaryOperationHintOf(node->op());
+        Type* rhs_type = GetUpperBound(node->InputAt(1));
+        VisitBinop(node, hint == BinaryOperationHints::kNumberOrOddball
+                             ? UseInfo::CheckedNumberOrOddballAsWord32()
+                             : UseInfo::CheckedSigned32AsWord32(),
+                   MachineRepresentation::kWord32, Type::Unsigned32());
+        if (lower()) {
+          lowering->DoShift(node, lowering->machine()->Word32Shr(), rhs_type);
+        }
+        return;
+      }
       case IrOpcode::kNumberAbs: {
         if (TypeOf(node->InputAt(0))->Is(Type::Unsigned32())) {
           VisitUnop(node, UseInfo::TruncatingWord32(),

src/compiler/simplified-operator.cc

@@ -267,7 +267,9 @@ BinaryOperationHints::Hint BinaryOperationHintOf(const Operator* op) {
          op->opcode() == IrOpcode::kSpeculativeNumberMultiply ||
          op->opcode() == IrOpcode::kSpeculativeNumberDivide ||
          op->opcode() == IrOpcode::kSpeculativeNumberModulus ||
-         op->opcode() == IrOpcode::kSpeculativeNumberShiftLeft);
+         op->opcode() == IrOpcode::kSpeculativeNumberShiftLeft ||
+         op->opcode() == IrOpcode::kSpeculativeNumberShiftRight ||
+         op->opcode() == IrOpcode::kSpeculativeNumberShiftRightLogical);
   return OpParameter<BinaryOperationHints::Hint>(op);
 }
 
@@ -363,7 +365,9 @@ CompareOperationHints::Hint CompareOperationHintOf(const Operator* op) {
   V(SpeculativeNumberDivide)      \
   V(SpeculativeNumberMultiply)    \
   V(SpeculativeNumberModulus)     \
-  V(SpeculativeNumberShiftLeft)
+  V(SpeculativeNumberShiftLeft)   \
+  V(SpeculativeNumberShiftRight)  \
+  V(SpeculativeNumberShiftRightLogical)
 
 #define CHECKED_OP_LIST(V)        \
   V(CheckBounds, 2, 1)            \

src/compiler/simplified-operator.h

@@ -240,6 +240,9 @@ class SimplifiedOperatorBuilder final : public ZoneObject {
   const Operator* SpeculativeNumberDivide(BinaryOperationHints::Hint hint);
   const Operator* SpeculativeNumberModulus(BinaryOperationHints::Hint hint);
   const Operator* SpeculativeNumberShiftLeft(BinaryOperationHints::Hint hint);
+  const Operator* SpeculativeNumberShiftRight(BinaryOperationHints::Hint hint);
+  const Operator* SpeculativeNumberShiftRightLogical(
+      BinaryOperationHints::Hint hint);
 
   const Operator* SpeculativeNumberLessThan(CompareOperationHints::Hint hint);
   const Operator* SpeculativeNumberLessThanOrEqual(

src/compiler/typer.cc

@@ -1697,6 +1697,14 @@ Type* Typer::Visitor::TypeSpeculativeNumberShiftLeft(Node* node) {
   return Type::Signed32();
 }
 
+Type* Typer::Visitor::TypeSpeculativeNumberShiftRight(Node* node) {
+  return Type::Signed32();
+}
+
+Type* Typer::Visitor::TypeSpeculativeNumberShiftRightLogical(Node* node) {
+  return Type::Unsigned32();
+}
+
 Type* Typer::Visitor::TypeNumberMultiply(Node* node) { return Type::Number(); }
 
 Type* Typer::Visitor::TypeNumberDivide(Node* node) { return Type::Number(); }

src/compiler/verifier.cc

@@ -721,6 +721,7 @@ void Verifier::Visitor::Check(Node* node) {
       CheckUpperIs(node, Type::Signed32());
       break;
     case IrOpcode::kSpeculativeNumberShiftLeft:
+    case IrOpcode::kSpeculativeNumberShiftRight:
       CheckUpperIs(node, Type::Signed32());
       break;
     case IrOpcode::kNumberShiftRightLogical:
@@ -729,6 +730,9 @@ void Verifier::Visitor::Check(Node* node) {
       CheckValueInputIs(node, 1, Type::Unsigned32());
       CheckUpperIs(node, Type::Unsigned32());
       break;
+    case IrOpcode::kSpeculativeNumberShiftRightLogical:
+      CheckUpperIs(node, Type::Unsigned32());
+      break;
     case IrOpcode::kNumberImul:
       // (Unsigned32, Unsigned32) -> Signed32
       CheckValueInputIs(node, 0, Type::Unsigned32());

test/mjsunit/compiler/turbo-number-feedback.js

@@ -78,3 +78,25 @@
   %OptimizeFunctionOnNextCall(f5);
   assertEquals(64, f5(4.9, 4.1));
 })();
+
+(function ShiftRightNumbers() {
+  function f6(a, b) {
+    return a >> b;
+  }
+
+  assertEquals(1, f6(8.3, 3.4));
+  assertEquals(-2, f6(-16.1, 3.9));
+  %OptimizeFunctionOnNextCall(f6);
+  assertEquals(0, f6(16.2, 5.1));
+})();
+
+(function ShiftRightLogicalNumbers() {
+  function f7(a, b) {
+    return a >>> b;
+  }
+
+  assertEquals(1, f7(8.3, 3.4));
+  assertEquals(536870910, f7(-16.1, 3.9));
+  %OptimizeFunctionOnNextCall(f7);
+  assertEquals(0, f7(16.2, 5.1));
+})();

test/unittests/compiler/js-typed-lowering-unittest.cc

@@ -448,6 +448,24 @@ TEST_F(JSTypedLoweringTest, JSShiftLeftWithSigned32AndUnsigned32) {
   EXPECT_THAT(r.replacement(), IsNumberShiftLeft(lhs, rhs));
 }
 
+TEST_F(JSTypedLoweringTest, JSShiftLeftWithTypeFeedback) {
+  BinaryOperationHints::Hint const feedback_types[] = {
+      BinaryOperationHints::kSignedSmall,
+      BinaryOperationHints::kNumberOrOddball};
+  for (BinaryOperationHints::Hint feedback : feedback_types) {
+    BinaryOperationHints const hints(feedback, feedback, feedback);
+    Node* lhs = Parameter(Type::Number(), 2);
+    Node* rhs = Parameter(Type::Number(), 3);
+    Node* effect = graph()->start();
+    Node* control = graph()->start();
+    Reduction r = Reduce(graph()->NewNode(
+        javascript()->ShiftLeft(hints), lhs, rhs, UndefinedConstant(),
+        EmptyFrameState(), EmptyFrameState(), effect, control));
+    ASSERT_TRUE(r.Changed());
+    EXPECT_THAT(r.replacement(), IsSpeculativeNumberShiftLeft(
+                                     feedback, lhs, rhs, effect, control));
+  }
+}
 
 // -----------------------------------------------------------------------------
 // JSShiftRight
@@ -484,6 +502,24 @@ TEST_F(JSTypedLoweringTest, JSShiftRightWithSigned32AndUnsigned32) {
   EXPECT_THAT(r.replacement(), IsNumberShiftRight(lhs, rhs));
 }
 
+TEST_F(JSTypedLoweringTest, JSShiftRightWithTypeFeedback) {
+  BinaryOperationHints::Hint const feedback_types[] = {
+      BinaryOperationHints::kSignedSmall,
+      BinaryOperationHints::kNumberOrOddball};
+  for (BinaryOperationHints::Hint feedback : feedback_types) {
+    BinaryOperationHints const hints(feedback, feedback, feedback);
+    Node* lhs = Parameter(Type::Number(), 2);
+    Node* rhs = Parameter(Type::Number(), 3);
+    Node* effect = graph()->start();
+    Node* control = graph()->start();
+    Reduction r = Reduce(graph()->NewNode(
+        javascript()->ShiftRight(hints), lhs, rhs, UndefinedConstant(),
+        EmptyFrameState(), EmptyFrameState(), effect, control));
+    ASSERT_TRUE(r.Changed());
+    EXPECT_THAT(r.replacement(), IsSpeculativeNumberShiftRight(
+                                     feedback, lhs, rhs, effect, control));
+  }
+}
 
 // -----------------------------------------------------------------------------
 // JSShiftRightLogical
@@ -521,6 +557,24 @@ TEST_F(JSTypedLoweringTest, JSShiftRightLogicalWithUnsigned32AndUnsigned32) {
   EXPECT_THAT(r.replacement(), IsNumberShiftRightLogical(lhs, rhs));
 }
 
+TEST_F(JSTypedLoweringTest, JSShiftRightLogicalWithTypeFeedback) {
+  BinaryOperationHints::Hint const feedback_types[] = {
+      BinaryOperationHints::kSignedSmall,
+      BinaryOperationHints::kNumberOrOddball};
+  for (BinaryOperationHints::Hint feedback : feedback_types) {
+    BinaryOperationHints const hints(feedback, feedback, feedback);
+    Node* lhs = Parameter(Type::Number(), 2);
+    Node* rhs = Parameter(Type::Number(), 3);
+    Node* effect = graph()->start();
+    Node* control = graph()->start();
+    Reduction r = Reduce(graph()->NewNode(
+        javascript()->ShiftRightLogical(hints), lhs, rhs, UndefinedConstant(),
+        EmptyFrameState(), EmptyFrameState(), effect, control));
+    ASSERT_TRUE(r.Changed());
+    EXPECT_THAT(r.replacement(), IsSpeculativeNumberShiftRightLogical(
+                                     feedback, lhs, rhs, effect, control));
+  }
+}
 
 // -----------------------------------------------------------------------------
 // JSLoadContext
@@ -890,43 +944,6 @@ TEST_F(JSTypedLoweringTest, JSSubtractSmis) {
                                           lhs, rhs, effect, control));
 }
 
-// -----------------------------------------------------------------------------
-// JSShiftLeft
-
-TEST_F(JSTypedLoweringTest, JSShiftLeftSmis) {
-  BinaryOperationHints const hints(BinaryOperationHints::kSignedSmall,
-                                   BinaryOperationHints::kSignedSmall,
-                                   BinaryOperationHints::kSignedSmall);
-  Node* lhs = Parameter(Type::Number(), 2);
-  Node* rhs = Parameter(Type::Number(), 3);
-  Node* effect = graph()->start();
-  Node* control = graph()->start();
-  Reduction r = Reduce(graph()->NewNode(
-      javascript()->ShiftLeft(hints), lhs, rhs, UndefinedConstant(),
-      EmptyFrameState(), EmptyFrameState(), effect, control));
-  ASSERT_TRUE(r.Changed());
-  EXPECT_THAT(r.replacement(),
-              IsSpeculativeNumberShiftLeft(BinaryOperationHints::kSignedSmall,
-                                           lhs, rhs, effect, control));
-}
-
-TEST_F(JSTypedLoweringTest, JSShiftLeftNumberOrOddball) {
-  BinaryOperationHints const hints(BinaryOperationHints::kNumberOrOddball,
-                                   BinaryOperationHints::kNumberOrOddball,
-                                   BinaryOperationHints::kNumberOrOddball);
-  Node* lhs = Parameter(Type::Number(), 2);
-  Node* rhs = Parameter(Type::Number(), 3);
-  Node* effect = graph()->start();
-  Node* control = graph()->start();
-  Reduction r = Reduce(graph()->NewNode(
-      javascript()->ShiftLeft(hints), lhs, rhs, UndefinedConstant(),
-      EmptyFrameState(), EmptyFrameState(), effect, control));
-  ASSERT_TRUE(r.Changed());
-  EXPECT_THAT(r.replacement(), IsSpeculativeNumberShiftLeft(
-                                   BinaryOperationHints::kNumberOrOddball, lhs,
-                                   rhs, effect, control));
-}
-
 // -----------------------------------------------------------------------------
 // JSInstanceOf
 // Test that instanceOf is reduced if and only if the right-hand side is a

test/unittests/compiler/node-test-utils.cc

@@ -809,6 +809,7 @@ class IsSpeculativeBinopMatcher final : public NodeMatcher {
       const Matcher<Node*>& effect_matcher,
       const Matcher<Node*>& control_matcher)
       : NodeMatcher(opcode),
+        hint_matcher_(hint_matcher),
         lhs_matcher_(lhs_matcher),
         rhs_matcher_(rhs_matcher),
         effect_matcher_(effect_matcher),
@@ -817,6 +818,9 @@ class IsSpeculativeBinopMatcher final : public NodeMatcher {
   bool MatchAndExplain(Node* node, MatchResultListener* listener) const final {
     return (NodeMatcher::MatchAndExplain(node, listener) &&
             // TODO(bmeurer): The type parameter is currently ignored.
+            PrintMatchAndExplain(
+                OpParameter<BinaryOperationHints::Hint>(node->op()), "hints",
+                hint_matcher_, listener) &&
             PrintMatchAndExplain(NodeProperties::GetValueInput(node, 0), "lhs",
                                  lhs_matcher_, listener) &&
             PrintMatchAndExplain(NodeProperties::GetValueInput(node, 1), "rhs",
@@ -828,6 +832,7 @@ class IsSpeculativeBinopMatcher final : public NodeMatcher {
   }
 
  private:
+  const Matcher<BinaryOperationHints::Hint> hint_matcher_;
   const Matcher<Type*> type_matcher_;
   const Matcher<Node*> lhs_matcher_;
   const Matcher<Node*> rhs_matcher_;
@@ -2068,6 +2073,26 @@ Matcher<Node*> IsSpeculativeNumberShiftLeft(
       rhs_matcher, effect_matcher, control_matcher));
 }
 
+Matcher<Node*> IsSpeculativeNumberShiftRight(
+    const Matcher<BinaryOperationHints::Hint>& hint_matcher,
+    const Matcher<Node*>& lhs_matcher, const Matcher<Node*>& rhs_matcher,
+    const Matcher<Node*>& effect_matcher,
+    const Matcher<Node*>& control_matcher) {
+  return MakeMatcher(new IsSpeculativeBinopMatcher(
+      IrOpcode::kSpeculativeNumberShiftRight, hint_matcher, lhs_matcher,
+      rhs_matcher, effect_matcher, control_matcher));
+}
+
+Matcher<Node*> IsSpeculativeNumberShiftRightLogical(
+    const Matcher<BinaryOperationHints::Hint>& hint_matcher,
+    const Matcher<Node*>& lhs_matcher, const Matcher<Node*>& rhs_matcher,
+    const Matcher<Node*>& effect_matcher,
+    const Matcher<Node*>& control_matcher) {
+  return MakeMatcher(new IsSpeculativeBinopMatcher(
+      IrOpcode::kSpeculativeNumberShiftRightLogical, hint_matcher, lhs_matcher,
+      rhs_matcher, effect_matcher, control_matcher));
+}
+
 Matcher<Node*> IsAllocate(const Matcher<Node*>& size_matcher,
                           const Matcher<Node*>& effect_matcher,
                           const Matcher<Node*>& control_matcher) {

test/unittests/compiler/node-test-utils.h

@@ -217,6 +217,16 @@ Matcher<Node*> IsSpeculativeNumberShiftLeft(
     const Matcher<Node*>& lhs_matcher, const Matcher<Node*>& rhs_matcher,
     const Matcher<Node*>& effect_matcher,
     const Matcher<Node*>& control_matcher);
+Matcher<Node*> IsSpeculativeNumberShiftRight(
+    const Matcher<BinaryOperationHints::Hint>& hint_matcher,
+    const Matcher<Node*>& lhs_matcher, const Matcher<Node*>& rhs_matcher,
+    const Matcher<Node*>& effect_matcher,
+    const Matcher<Node*>& control_matcher);
+Matcher<Node*> IsSpeculativeNumberShiftRightLogical(
+    const Matcher<BinaryOperationHints::Hint>& hint_matcher,
+    const Matcher<Node*>& lhs_matcher, const Matcher<Node*>& rhs_matcher,
+    const Matcher<Node*>& effect_matcher,
+    const Matcher<Node*>& control_matcher);
 Matcher<Node*> IsNumberSubtract(const Matcher<Node*>& lhs_matcher,
                                 const Matcher<Node*>& rhs_matcher);
 Matcher<Node*> IsNumberMultiply(const Matcher<Node*>& lhs_matcher,