[nci] Add feedback vector as input to unary ops

In native context independent code we cannot embed the (native context
dependent) feedback vector as a constant. Instead, we will load it
from the JSFunction once and pass it to all users. This CL makes this
change for all unary operators. All other {binary,compare} operators
will need similar work in the future.

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

src/compiler/access-builder.cc

@@ -1168,6 +1168,15 @@ FieldAccess AccessBuilder::ForDictionaryObjectHashIndex() {
   return access;
 }
 
+// static
+FieldAccess AccessBuilder::ForFeedbackCellValue() {
+  FieldAccess access = {kTaggedBase,      FeedbackCell::kValueOffset,
+                        Handle<Name>(),   MaybeHandle<Map>(),
+                        Type::Any(),      MachineType::TaggedPointer(),
+                        kFullWriteBarrier};
+  return access;
+}
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

src/compiler/access-builder.h

@@ -327,6 +327,9 @@ class V8_EXPORT_PRIVATE AccessBuilder final
   static FieldAccess ForDictionaryNextEnumerationIndex();
   static FieldAccess ForDictionaryObjectHashIndex();
 
+  // Provides access to a FeedbackCell's value.
+  static FieldAccess ForFeedbackCellValue();
+
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(AccessBuilder);
 };

src/compiler/bytecode-graph-builder.cc

@@ -63,6 +63,23 @@ class BytecodeGraphBuilder {
   // Get or create the node that represents the outer function closure.
   Node* GetFunctionClosure();
 
+  bool native_context_independent() const {
+    return native_context_independent_;
+  }
+
+  // The node representing the current feedback vector is generated once prior
+  // to visiting bytecodes, and is later passed as input to other nodes that
+  // may need it.
+  // TODO(jgruber): Remove feedback_vector() and rename feedback_vector_node()
+  // to feedback_vector() once all uses of the direct heap object reference
+  // have been replaced with a Node* reference.
+  void CreateFeedbackVectorNode();
+  Node* BuildLoadFeedbackVector();
+  Node* feedback_vector_node() const {
+    DCHECK_NOT_NULL(feedback_vector_node_);
+    return feedback_vector_node_;
+  }
+
   // Builder for loading the a native context field.
   Node* BuildLoadNativeContextField(int index);
 
@@ -416,6 +433,9 @@ class BytecodeGraphBuilder {
   int input_buffer_size_;
   Node** input_buffer_;
 
+  const bool native_context_independent_;
+  Node* feedback_vector_node_;
+
   // Optimization to only create checkpoints when the current position in the
   // control-flow is not effect-dominated by another checkpoint already. All
   // operations that do not have observable side-effects can be re-evaluated.
@@ -978,6 +998,9 @@ BytecodeGraphBuilder::BytecodeGraphBuilder(
       current_exception_handler_(0),
       input_buffer_size_(0),
       input_buffer_(nullptr),
+      native_context_independent_(
+          flags & BytecodeGraphBuilderFlag::kNativeContextIndependent),
+      feedback_vector_node_(nullptr),
       needs_eager_checkpoint_(true),
       exit_controls_(local_zone),
       state_values_cache_(jsgraph),
@@ -1008,6 +1031,36 @@ Node* BytecodeGraphBuilder::GetFunctionClosure() {
   return function_closure_.get();
 }
 
+void BytecodeGraphBuilder::CreateFeedbackVectorNode() {
+  DCHECK_NULL(feedback_vector_node_);
+  feedback_vector_node_ = native_context_independent()
+                              ? BuildLoadFeedbackVector()
+                              : jsgraph()->Constant(feedback_vector());
+}
+
+Node* BytecodeGraphBuilder::BuildLoadFeedbackVector() {
+  DCHECK(native_context_independent());
+  DCHECK_NULL(feedback_vector_node_);
+
+  // The feedback vector must exist and remain live while the generated code
+  // lives. Specifically that means it must be created when NCI code is
+  // installed, and must not be flushed.
+
+  Environment* env = environment();
+  Node* control = env->GetControlDependency();
+  Node* effect = env->GetEffectDependency();
+
+  Node* feedback_cell = effect = graph()->NewNode(
+      simplified()->LoadField(AccessBuilder::ForJSFunctionFeedbackCell()),
+      GetFunctionClosure(), effect, control);
+  Node* vector = effect = graph()->NewNode(
+      simplified()->LoadField(AccessBuilder::ForFeedbackCellValue()),
+      feedback_cell, effect, control);
+
+  env->UpdateEffectDependency(effect);
+  return vector;
+}
+
 Node* BytecodeGraphBuilder::BuildLoadNativeContextField(int index) {
   Node* result = NewNode(javascript()->LoadContext(0, index, true));
   NodeProperties::ReplaceContextInput(result,
@@ -1039,6 +1092,7 @@ void BytecodeGraphBuilder::CreateGraph() {
                   graph()->start());
   set_environment(&env);
 
+  CreateFeedbackVectorNode();
   VisitBytecodes();
 
   // Finish the basic structure of the graph.
@@ -2719,7 +2773,7 @@ void BytecodeGraphBuilder::BuildUnaryOp(const Operator* op) {
     node = lowering.value();
   } else {
     DCHECK(!lowering.Changed());
-    node = NewNode(op, operand);
+    node = NewNode(op, operand, feedback_vector_node());
   }
 
   environment()->BindAccumulator(node, Environment::kAttachFrameState);

src/compiler/graph-assembler.h

@@ -133,16 +133,6 @@ class GraphAssembler;
 // Wrapper classes for special node/edge types (effect, control, frame states)
 // that otherwise don't fit into the type system.
 
-class NodeWrapper {
- public:
-  explicit constexpr NodeWrapper(Node* node) : node_(node) {}
-  operator Node*() const { return node_; }
-  Node* operator->() const { return node_; }
-
- private:
-  Node* node_;
-};
-
 class Effect : public NodeWrapper {
  public:
   explicit constexpr Effect(Node* node) : NodeWrapper(node) {

src/compiler/js-generic-lowering.cc

@@ -130,7 +130,6 @@ void JSGenericLowering::ReplaceUnaryOpWithBuiltinCall(
   const FeedbackParameter& p = FeedbackParameterOf(node->op());
   if (CollectFeedbackInGenericLowering() && p.feedback().IsValid()) {
     Callable callable = Builtins::CallableFor(isolate(), builtin_with_feedback);
-    Node* feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
     Node* slot = jsgraph()->UintPtrConstant(p.feedback().slot.ToInt());
     const CallInterfaceDescriptor& descriptor = callable.descriptor();
     CallDescriptor::Flags flags = FrameStateFlagForCall(node);
@@ -138,11 +137,14 @@ void JSGenericLowering::ReplaceUnaryOpWithBuiltinCall(
         zone(), descriptor, descriptor.GetStackParameterCount(), flags,
         node->op()->properties());
     Node* stub_code = jsgraph()->HeapConstant(callable.code());
+    STATIC_ASSERT(JSUnaryOpNode::ValueIndex() == 0);
+    STATIC_ASSERT(JSUnaryOpNode::FeedbackVectorIndex() == 1);
+    DCHECK_EQ(node->op()->ValueInputCount(), 2);
     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 {
+    node->RemoveInput(JSUnaryOpNode::FeedbackVectorIndex());
     ReplaceWithBuiltinCall(node, builtin_without_feedback);
   }
 }

src/compiler/js-operator.cc

@@ -7,7 +7,6 @@
 #include <limits>
 
 #include "src/base/lazy-instance.h"
-#include "src/compiler/opcodes.h"
 #include "src/compiler/operator.h"
 #include "src/handles/handles-inl.h"
 #include "src/objects/objects-inl.h"
@@ -737,7 +736,7 @@ CACHED_OP_LIST(CACHED_OP)
   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, \
+        IrOpcode::kJS##Name, Operator::kNoProperties, "JS" #Name, 2, 1, 1, 1, \
         1, 2, parameters);                                                    \
   }
 UNARY_OP_LIST(UNARY_OP)

src/compiler/js-operator.h

@@ -8,6 +8,8 @@
 #include "src/base/compiler-specific.h"
 #include "src/compiler/feedback-source.h"
 #include "src/compiler/globals.h"
+#include "src/compiler/node.h"
+#include "src/compiler/opcodes.h"
 #include "src/handles/maybe-handles.h"
 #include "src/objects/type-hints.h"
 #include "src/runtime/runtime.h"
@@ -27,6 +29,26 @@ namespace compiler {
 class Operator;
 struct JSOperatorGlobalCache;
 
+// Node wrappers.
+
+class JSUnaryOpNode final : public NodeWrapper {
+ public:
+  explicit constexpr JSUnaryOpNode(Node* node) : NodeWrapper(node) {
+    CONSTEXPR_DCHECK(node->opcode() == IrOpcode::kJSBitwiseNot ||
+                     node->opcode() == IrOpcode::kJSDecrement ||
+                     node->opcode() == IrOpcode::kJSIncrement ||
+                     node->opcode() == IrOpcode::kJSNegate);
+  }
+
+  static constexpr int ValueIndex() { return 0; }
+  static constexpr int FeedbackVectorIndex() { return 1; }
+};
+
+using JSBitwiseNotNode = JSUnaryOpNode;
+using JSDecrementNode = JSUnaryOpNode;
+using JSIncrementNode = JSUnaryOpNode;
+using JSNegateNode = JSUnaryOpNode;
+
 // Defines the frequency a given Call/Construct site was executed. For some
 // call sites the frequency is not known.
 class CallFrequency final {

src/compiler/js-typed-lowering.cc

@@ -455,6 +455,7 @@ Reduction JSTypedLowering::ReduceJSBitwiseNot(Node* node) {
   if (input_type.Is(Type::PlainPrimitive())) {
     // JSBitwiseNot(x) => NumberBitwiseXor(ToInt32(x), -1)
     const FeedbackParameter& p = FeedbackParameterOf(node->op());
+    node->RemoveInput(JSBitwiseNotNode::FeedbackVectorIndex());
     node->InsertInput(graph()->zone(), 1, jsgraph()->SmiConstant(-1));
     NodeProperties::ChangeOp(node, javascript()->BitwiseXor(p.feedback()));
     JSBinopReduction r(this, node);
@@ -471,6 +472,7 @@ Reduction JSTypedLowering::ReduceJSDecrement(Node* node) {
   if (input_type.Is(Type::PlainPrimitive())) {
     // JSDecrement(x) => NumberSubtract(ToNumber(x), 1)
     const FeedbackParameter& p = FeedbackParameterOf(node->op());
+    node->RemoveInput(JSDecrementNode::FeedbackVectorIndex());
     node->InsertInput(graph()->zone(), 1, jsgraph()->OneConstant());
     NodeProperties::ChangeOp(node, javascript()->Subtract(p.feedback()));
     JSBinopReduction r(this, node);
@@ -487,6 +489,7 @@ Reduction JSTypedLowering::ReduceJSIncrement(Node* node) {
   if (input_type.Is(Type::PlainPrimitive())) {
     // JSIncrement(x) => NumberAdd(ToNumber(x), 1)
     const FeedbackParameter& p = FeedbackParameterOf(node->op());
+    node->RemoveInput(JSIncrementNode::FeedbackVectorIndex());
     node->InsertInput(graph()->zone(), 1, jsgraph()->OneConstant());
     NodeProperties::ChangeOp(node, javascript()->Add(p.feedback()));
     JSBinopReduction r(this, node);
@@ -503,6 +506,7 @@ Reduction JSTypedLowering::ReduceJSNegate(Node* node) {
   if (input_type.Is(Type::PlainPrimitive())) {
     // JSNegate(x) => NumberMultiply(ToNumber(x), -1)
     const FeedbackParameter& p = FeedbackParameterOf(node->op());
+    node->RemoveInput(JSNegateNode::FeedbackVectorIndex());
     node->InsertInput(graph()->zone(), 1, jsgraph()->SmiConstant(-1));
     NodeProperties::ChangeOp(node, javascript()->Multiply(p.feedback()));
     JSBinopReduction r(this, node);

src/compiler/node.h

@@ -303,6 +303,16 @@ Node** Node::OutOfLineInputs::inputs() {
 
 std::ostream& operator<<(std::ostream& os, const Node& n);
 
+// Base class for node wrappers.
+class NodeWrapper {
+ public:
+  explicit constexpr NodeWrapper(Node* node) : node_(node) {}
+  operator Node*() const { return node_; }
+  Node* operator->() const { return node_; }
+
+ private:
+  Node* node_;
+};
 
 // Typedefs to shorten commonly used Node containers.
 using NodeDeque = ZoneDeque<Node*>;