[compiler] Hook in unary op builtins with feedback in generic lowering

If --turbo-nci is enabled, use unary op builtins with feedback
collection during generic lowering.

Bug: v8:8888
Change-Id: Ie32cfe1558a7fbada2ac69a99ef969097558bc89
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2209067
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67962}

src/compiler/bytecode-graph-builder.cc

@@ -2810,19 +2810,27 @@ SpeculationMode BytecodeGraphBuilder::GetSpeculationMode(int slot_id) const {
 }
 
 void BytecodeGraphBuilder::VisitBitwiseNot() {
-  BuildUnaryOp(javascript()->BitwiseNot());
+  FeedbackSource feedback = CreateFeedbackSource(
+      bytecode_iterator().GetSlotOperand(kUnaryOperationHintIndex).ToInt());
+  BuildUnaryOp(javascript()->BitwiseNot(feedback));
 }
 
 void BytecodeGraphBuilder::VisitDec() {
-  BuildUnaryOp(javascript()->Decrement());
+  FeedbackSource feedback = CreateFeedbackSource(
+      bytecode_iterator().GetSlotOperand(kUnaryOperationHintIndex).ToInt());
+  BuildUnaryOp(javascript()->Decrement(feedback));
 }
 
 void BytecodeGraphBuilder::VisitInc() {
-  BuildUnaryOp(javascript()->Increment());
+  FeedbackSource feedback = CreateFeedbackSource(
+      bytecode_iterator().GetSlotOperand(kUnaryOperationHintIndex).ToInt());
+  BuildUnaryOp(javascript()->Increment(feedback));
 }
 
 void BytecodeGraphBuilder::VisitNegate() {
-  BuildUnaryOp(javascript()->Negate());
+  FeedbackSource feedback = CreateFeedbackSource(
+      bytecode_iterator().GetSlotOperand(kUnaryOperationHintIndex).ToInt());
+  BuildUnaryOp(javascript()->Negate(feedback));
 }
 
 void BytecodeGraphBuilder::VisitAdd() {

src/compiler/globals.h

@@ -6,11 +6,22 @@
 #define V8_COMPILER_GLOBALS_H_
 
 #include "src/common/globals.h"
+#include "src/flags/flags.h"
 
 namespace v8 {
 namespace internal {
 namespace compiler {
 
+// The nci flag is currently used to experiment with feedback collection in
+// optimized code produced by generic lowering.
+// Considerations:
+// - Should we increment the call count? https://crbug.com/v8/10524
+// - Is feedback already megamorphic in all these cases?
+//
+// TODO(jgruber): Remove once we've made a decision whether to collect feedback
+// unconditionally.
+inline bool CollectFeedbackInGenericLowering() { return FLAG_turbo_nci; }
+
 enum class StackCheckKind {
   kJSFunctionEntry = 0,
   kJSIterationBody,

src/compiler/js-generic-lowering.cc

@@ -25,16 +25,6 @@ namespace compiler {
 
 namespace {
 
-// The nci flag is currently used to experiment with feedback collection in
-// optimized code produced by generic lowering.
-// Considerations:
-// - Should we increment the call count? https://crbug.com/v8/10524
-// - Is feedback already megamorphic in all these cases?
-//
-// TODO(jgruber): Remove once we've made a decision whether to collect feedback
-// unconditionally.
-bool CollectFeedback() { return FLAG_turbo_nci; }
-
 CallDescriptor::Flags FrameStateFlagForCall(Node* node) {
   return OperatorProperties::HasFrameStateInput(node->op())
              ? CallDescriptor::kNeedsFrameState
@@ -87,10 +77,6 @@ REPLACE_STUB_CALL(LessThan)
 REPLACE_STUB_CALL(LessThanOrEqual)
 REPLACE_STUB_CALL(GreaterThan)
 REPLACE_STUB_CALL(GreaterThanOrEqual)
-REPLACE_STUB_CALL(BitwiseNot)
-REPLACE_STUB_CALL(Decrement)
-REPLACE_STUB_CALL(Increment)
-REPLACE_STUB_CALL(Negate)
 REPLACE_STUB_CALL(HasProperty)
 REPLACE_STUB_CALL(Equal)
 REPLACE_STUB_CALL(ToLength)
@@ -111,6 +97,8 @@ REPLACE_STUB_CALL(RejectPromise)
 REPLACE_STUB_CALL(ResolvePromise)
 #undef REPLACE_STUB_CALL
 
+// TODO(jgruber): Hook in binary and compare op builtins with feedback.
+
 void JSGenericLowering::ReplaceWithStubCall(Node* node,
                                             Callable callable,
                                             CallDescriptor::Flags flags) {
@@ -157,6 +145,42 @@ void JSGenericLowering::LowerJSStrictEqual(Node* node) {
                       Operator::kEliminatable);
 }
 
+void JSGenericLowering::ReplaceUnaryOpWithBuiltinCall(
+    Node* node, Builtins::Name builtin_without_feedback,
+    Builtins::Name builtin_with_feedback) {
+  const FeedbackParameter& p = FeedbackParameterOf(node->op());
+  CallDescriptor::Flags flags = FrameStateFlagForCall(node);
+  if (CollectFeedbackInGenericLowering() && p.feedback().IsValid()) {
+    Callable callable = Builtins::CallableFor(isolate(), builtin_with_feedback);
+    Node* feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
+    Node* slot = jsgraph()->Int32Constant(p.feedback().slot.ToInt());
+    const CallInterfaceDescriptor& descriptor = callable.descriptor();
+    auto call_descriptor = Linkage::GetStubCallDescriptor(
+        zone(), descriptor, descriptor.GetStackParameterCount(), flags,
+        node->op()->properties());
+    Node* stub_code = jsgraph()->HeapConstant(callable.code());
+    node->InsertInput(zone(), 0, stub_code);
+    node->InsertInput(zone(), 2, slot);
+    node->InsertInput(zone(), 3, feedback_vector);
+    NodeProperties::ChangeOp(node, common()->Call(call_descriptor));
+  } else {
+    Callable callable =
+        Builtins::CallableFor(isolate(), builtin_without_feedback);
+    ReplaceWithStubCall(node, callable, flags);
+  }
+}
+
+#define DEF_UNARY_LOWERING(Name)                                     \
+  void JSGenericLowering::LowerJS##Name(Node* node) {                \
+    ReplaceUnaryOpWithBuiltinCall(node, Builtins::k##Name,           \
+                                  Builtins::k##Name##_WithFeedback); \
+  }
+DEF_UNARY_LOWERING(BitwiseNot)
+DEF_UNARY_LOWERING(Decrement)
+DEF_UNARY_LOWERING(Increment)
+DEF_UNARY_LOWERING(Negate)
+#undef DEF_UNARY_LOWERING
+
 namespace {
 bool ShouldUseMegamorphicLoadBuiltin(FeedbackSource const& source,
                                      JSHeapBroker* broker) {
@@ -709,14 +733,14 @@ void JSGenericLowering::LowerJSConstruct(Node* node) {
   static constexpr int kReceiver = 1;
   static constexpr int kMaybeFeedbackVector = 1;
 
-  if (CollectFeedback() && p.feedback().IsValid()) {
+  if (CollectFeedbackInGenericLowering() && p.feedback().IsValid()) {
     const int stack_argument_count =
         arg_count + kReceiver + kMaybeFeedbackVector;
     Callable callable =
         Builtins::CallableFor(isolate(), Builtins::kConstruct_WithFeedback);
     auto call_descriptor = Linkage::GetStubCallDescriptor(
         zone(), callable.descriptor(), stack_argument_count, flags);
-    Node* maybe_feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
+    Node* feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
     Node* stub_code = jsgraph()->HeapConstant(callable.code());
     Node* new_target = node->InputAt(arg_count + 1);
     Node* stub_arity = jsgraph()->Int32Constant(arg_count);
@@ -724,11 +748,11 @@ void JSGenericLowering::LowerJSConstruct(Node* node) {
     Node* receiver = jsgraph()->UndefinedConstant();
     node->RemoveInput(arg_count + 1);  // Drop new target.
     // Register argument inputs are followed by stack argument inputs (such as
-    // maybe_feedback_vector). Both are listed in ascending order. Note that
+    // feedback_vector). Both are listed in ascending order. Note that
     // the receiver is implicitly placed on the stack and is thus inserted
     // between explicitly-specified register and stack arguments.
     // TODO(jgruber): Implement a simpler way to specify these mutations.
-    node->InsertInput(zone(), arg_count + 1, maybe_feedback_vector);
+    node->InsertInput(zone(), arg_count + 1, feedback_vector);
     node->InsertInput(zone(), 0, stub_code);
     node->InsertInput(zone(), 2, new_target);
     node->InsertInput(zone(), 3, stub_arity);
@@ -763,7 +787,7 @@ void JSGenericLowering::LowerJSConstructWithArrayLike(Node* node) {
   static constexpr int kArgumentList = 1;
   static constexpr int kMaybeFeedbackVector = 1;
 
-  if (CollectFeedback() && p.feedback().IsValid()) {
+  if (CollectFeedbackInGenericLowering() && p.feedback().IsValid()) {
     const int stack_argument_count =
         arg_count - kArgumentList + kReceiver + kMaybeFeedbackVector;
     Callable callable = Builtins::CallableFor(
@@ -774,20 +798,20 @@ void JSGenericLowering::LowerJSConstructWithArrayLike(Node* node) {
     Node* receiver = jsgraph()->UndefinedConstant();
     Node* arguments_list = node->InputAt(1);
     Node* new_target = node->InputAt(2);
-    Node* maybe_feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
+    Node* feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
     Node* slot = jsgraph()->Int32Constant(p.feedback().index());
 
     node->InsertInput(zone(), 0, stub_code);
     node->ReplaceInput(2, new_target);
     node->ReplaceInput(3, arguments_list);
     // Register argument inputs are followed by stack argument inputs (such as
-    // maybe_feedback_vector). Both are listed in ascending order. Note that
+    // feedback_vector). Both are listed in ascending order. Note that
     // the receiver is implicitly placed on the stack and is thus inserted
     // between explicitly-specified register and stack arguments.
     // TODO(jgruber): Implement a simpler way to specify these mutations.
     node->InsertInput(zone(), 4, slot);
     node->InsertInput(zone(), 5, receiver);
-    node->InsertInput(zone(), 6, maybe_feedback_vector);
+    node->InsertInput(zone(), 6, feedback_vector);
 
     NodeProperties::ChangeOp(node, common()->Call(call_descriptor));
   } else {
@@ -819,7 +843,7 @@ void JSGenericLowering::LowerJSConstructWithSpread(Node* node) {
   static constexpr int kTheSpread = 1;  // Included in `arg_count`.
   static constexpr int kMaybeFeedbackVector = 1;
 
-  if (CollectFeedback() && p.feedback().IsValid()) {
+  if (CollectFeedbackInGenericLowering() && p.feedback().IsValid()) {
     const int stack_argument_count =
         arg_count + kReceiver + kMaybeFeedbackVector;
     Callable callable = Builtins::CallableFor(
@@ -827,7 +851,7 @@ void JSGenericLowering::LowerJSConstructWithSpread(Node* node) {
     auto call_descriptor = Linkage::GetStubCallDescriptor(
         zone(), callable.descriptor(), stack_argument_count, flags);
     Node* stub_code = jsgraph()->HeapConstant(callable.code());
-    Node* maybe_feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
+    Node* feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
     Node* slot = jsgraph()->Int32Constant(p.feedback().index());
 
     // The single available register is needed for `slot`, thus `spread` remains
@@ -838,11 +862,11 @@ void JSGenericLowering::LowerJSConstructWithSpread(Node* node) {
     node->RemoveInput(new_target_index);
 
     // Register argument inputs are followed by stack argument inputs (such as
-    // maybe_feedback_vector). Both are listed in ascending order. Note that
+    // feedback_vector). Both are listed in ascending order. Note that
     // the receiver is implicitly placed on the stack and is thus inserted
     // between explicitly-specified register and stack arguments.
     // TODO(jgruber): Implement a simpler way to specify these mutations.
-    node->InsertInput(zone(), arg_count + 1, maybe_feedback_vector);
+    node->InsertInput(zone(), arg_count + 1, feedback_vector);
     node->InsertInput(zone(), 0, stub_code);
     node->InsertInput(zone(), 2, new_target);
     node->InsertInput(zone(), 3, stub_arity);
@@ -895,19 +919,19 @@ void JSGenericLowering::LowerJSCall(Node* node) {
   int const arg_count = static_cast<int>(p.arity() - 2);
   ConvertReceiverMode const mode = p.convert_mode();
 
-  if (CollectFeedback() && p.feedback().IsValid()) {
+  if (CollectFeedbackInGenericLowering() && p.feedback().IsValid()) {
     Callable callable = CodeFactory::Call_WithFeedback(isolate(), mode);
     CallDescriptor::Flags flags = FrameStateFlagForCall(node);
     auto call_descriptor = Linkage::GetStubCallDescriptor(
         zone(), callable.descriptor(), arg_count + 1, flags);
     Node* stub_code = jsgraph()->HeapConstant(callable.code());
     Node* stub_arity = jsgraph()->Int32Constant(arg_count);
-    Node* maybe_feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
+    Node* feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
     Node* slot = jsgraph()->Int32Constant(p.feedback().index());
     node->InsertInput(zone(), 0, stub_code);
     node->InsertInput(zone(), 2, stub_arity);
     node->InsertInput(zone(), 3, slot);
-    node->InsertInput(zone(), 4, maybe_feedback_vector);
+    node->InsertInput(zone(), 4, feedback_vector);
     NodeProperties::ChangeOp(node, common()->Call(call_descriptor));
   } else {
     Callable callable = CodeFactory::Call(isolate(), mode);
@@ -930,7 +954,7 @@ void JSGenericLowering::LowerJSCallWithArrayLike(Node* node) {
   DCHECK_EQ(arg_count, 0);
   static constexpr int kReceiver = 1;
 
-  if (CollectFeedback() && p.feedback().IsValid()) {
+  if (CollectFeedbackInGenericLowering() && p.feedback().IsValid()) {
     Callable callable = Builtins::CallableFor(
         isolate(), Builtins::kCallWithArrayLike_WithFeedback);
     auto call_descriptor = Linkage::GetStubCallDescriptor(
@@ -938,13 +962,13 @@ void JSGenericLowering::LowerJSCallWithArrayLike(Node* node) {
     Node* stub_code = jsgraph()->HeapConstant(callable.code());
     Node* receiver = node->InputAt(1);
     Node* arguments_list = node->InputAt(2);
-    Node* maybe_feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
+    Node* feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
     Node* slot = jsgraph()->Int32Constant(p.feedback().index());
     node->InsertInput(zone(), 0, stub_code);
     node->ReplaceInput(2, arguments_list);
     node->ReplaceInput(3, receiver);
     node->InsertInput(zone(), 3, slot);
-    node->InsertInput(zone(), 4, maybe_feedback_vector);
+    node->InsertInput(zone(), 4, feedback_vector);
     NodeProperties::ChangeOp(node, common()->Call(call_descriptor));
   } else {
     Callable callable = CodeFactory::CallWithArrayLike(isolate());
@@ -969,14 +993,14 @@ void JSGenericLowering::LowerJSCallWithSpread(Node* node) {
   static constexpr int kTheSpread = 1;
   static constexpr int kMaybeFeedbackVector = 1;
 
-  if (CollectFeedback() && p.feedback().IsValid()) {
+  if (CollectFeedbackInGenericLowering() && p.feedback().IsValid()) {
     const int stack_argument_count = arg_count + kMaybeFeedbackVector;
     Callable callable = Builtins::CallableFor(
         isolate(), Builtins::kCallWithSpread_WithFeedback);
     auto call_descriptor = Linkage::GetStubCallDescriptor(
         zone(), callable.descriptor(), stack_argument_count, flags);
     Node* stub_code = jsgraph()->HeapConstant(callable.code());
-    Node* maybe_feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
+    Node* feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
     Node* slot = jsgraph()->Int32Constant(p.feedback().index());
 
     // We pass the spread in a register, not on the stack.
@@ -985,11 +1009,11 @@ void JSGenericLowering::LowerJSCallWithSpread(Node* node) {
     node->RemoveInput(spread_index);
 
     // Register argument inputs are followed by stack argument inputs (such as
-    // maybe_feedback_vector). Both are listed in ascending order. Note that
+    // feedback_vector). Both are listed in ascending order. Note that
     // the receiver is implicitly placed on the stack and is thus inserted
     // between explicitly-specified register and stack arguments.
     // TODO(jgruber): Implement a simpler way to specify these mutations.
-    node->InsertInput(zone(), arg_count + 1, maybe_feedback_vector);
+    node->InsertInput(zone(), arg_count + 1, feedback_vector);
     node->InsertInput(zone(), 0, stub_code);
     node->InsertInput(zone(), 2, stub_arity);
     node->InsertInput(zone(), 3, spread);

src/compiler/js-generic-lowering.h

@@ -42,6 +42,10 @@ class JSGenericLowering final : public AdvancedReducer {
                            Operator::Properties properties);
   void ReplaceWithRuntimeCall(Node* node, Runtime::FunctionId f, int args = -1);
 
+  void ReplaceUnaryOpWithBuiltinCall(Node* node,
+                                     Builtins::Name builtin_without_feedback,
+                                     Builtins::Name builtin_with_feedback);
+
   Zone* zone() const;
   Isolate* isolate() const;
   JSGraph* jsgraph() const { return jsgraph_; }

src/compiler/js-operator.cc

@@ -226,8 +226,12 @@ std::ostream& operator<<(std::ostream& os, FeedbackParameter const& p) {
 }
 
 FeedbackParameter const& FeedbackParameterOf(const Operator* op) {
-  DCHECK(op->opcode() == IrOpcode::kJSCreateEmptyLiteralArray ||
+  DCHECK(op->opcode() == IrOpcode::kJSDecrement ||
+         op->opcode() == IrOpcode::kJSBitwiseNot ||
+         op->opcode() == IrOpcode::kJSCreateEmptyLiteralArray ||
+         op->opcode() == IrOpcode::kJSIncrement ||
          op->opcode() == IrOpcode::kJSInstanceOf ||
+         op->opcode() == IrOpcode::kJSNegate ||
          op->opcode() == IrOpcode::kJSStoreDataPropertyInLiteral ||
          op->opcode() == IrOpcode::kJSStoreInArrayLiteral);
   return OpParameter<FeedbackParameter>(op);
@@ -659,10 +663,6 @@ CompareOperationHint CompareOperationHintOf(const Operator* op) {
   V(Divide, Operator::kNoProperties, 2, 1)                               \
   V(Modulus, Operator::kNoProperties, 2, 1)                              \
   V(Exponentiate, Operator::kNoProperties, 2, 1)                         \
-  V(BitwiseNot, Operator::kNoProperties, 1, 1)                           \
-  V(Decrement, Operator::kNoProperties, 1, 1)                            \
-  V(Increment, Operator::kNoProperties, 1, 1)                            \
-  V(Negate, Operator::kNoProperties, 1, 1)                               \
   V(ToLength, Operator::kNoProperties, 1, 1)                             \
   V(ToName, Operator::kNoProperties, 1, 1)                               \
   V(ToNumber, Operator::kNoProperties, 1, 1)                             \
@@ -699,6 +699,12 @@ CompareOperationHint CompareOperationHintOf(const Operator* op) {
   V(ParseInt, Operator::kNoProperties, 2, 1)                             \
   V(RegExpTest, Operator::kNoProperties, 2, 1)
 
+#define UNARY_OP_LIST(V) \
+  V(BitwiseNot)          \
+  V(Decrement)           \
+  V(Increment)           \
+  V(Negate)
+
 #define BINARY_OP_LIST(V) V(Add)
 
 #define COMPARE_OP_LIST(V)                    \
@@ -815,6 +821,16 @@ CACHED_OP_LIST(CACHED_OP)
 BINARY_OP_LIST(BINARY_OP)
 #undef BINARY_OP
 
+#define UNARY_OP(Name)                                                        \
+  const Operator* JSOperatorBuilder::Name(FeedbackSource const& feedback) {   \
+    FeedbackParameter parameters(feedback);                                   \
+    return new (zone()) Operator1<FeedbackParameter>(                         \
+        IrOpcode::kJS##Name, Operator::kNoProperties, "JS" #Name, 1, 1, 1, 1, \
+        1, 2, parameters);                                                    \
+  }
+UNARY_OP_LIST(UNARY_OP)
+#undef UNARY_OP
+
 #define COMPARE_OP(Name, ...)                                          \
   const Operator* JSOperatorBuilder::Name(CompareOperationHint hint) { \
     switch (hint) {                                                    \
@@ -1418,6 +1434,7 @@ Handle<ScopeInfo> ScopeInfoOf(const Operator* op) {
   return OpParameter<Handle<ScopeInfo>>(op);
 }
 
+#undef UNARY_OP_LIST
 #undef BINARY_OP_LIST
 #undef CACHED_OP_LIST
 #undef COMPARE_OP_LIST

src/compiler/js-operator.h

@@ -771,10 +771,10 @@ class V8_EXPORT_PRIVATE JSOperatorBuilder final
   const Operator* Modulus();
   const Operator* Exponentiate();
 
-  const Operator* BitwiseNot();
-  const Operator* Decrement();
-  const Operator* Increment();
-  const Operator* Negate();
+  const Operator* BitwiseNot(FeedbackSource const& feedback);
+  const Operator* Decrement(FeedbackSource const& feedback);
+  const Operator* Increment(FeedbackSource const& feedback);
+  const Operator* Negate(FeedbackSource const& feedback);
 
   const Operator* ToLength();
   const Operator* ToName();