From fc3c197562fdc506577f032cc4548d47d47ca53a Mon Sep 17 00:00:00 2001 From: Jakob Gruber Date: Thu, 18 Jun 2020 12:47:11 +0200 Subject: [PATCH] [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/+/2246576 Reviewed-by: Georg Neis Commit-Queue: Jakob Gruber Cr-Commit-Position: refs/heads/master@{#68448} --- src/compiler/access-builder.cc | 9 +++++ src/compiler/access-builder.h | 3 ++ src/compiler/bytecode-graph-builder.cc | 56 +++++++++++++++++++++++++- src/compiler/graph-assembler.h | 10 ----- src/compiler/js-generic-lowering.cc | 6 ++- src/compiler/js-operator.cc | 3 +- src/compiler/js-operator.h | 22 ++++++++++ src/compiler/js-typed-lowering.cc | 4 ++ src/compiler/node.h | 10 +++++ 9 files changed, 108 insertions(+), 15 deletions(-) diff --git a/src/compiler/access-builder.cc b/src/compiler/access-builder.cc index 09603fd16c..656b250a1c 100644 --- a/src/compiler/access-builder.cc +++ b/src/compiler/access-builder.cc @@ -1168,6 +1168,15 @@ FieldAccess AccessBuilder::ForDictionaryObjectHashIndex() { return access; } +// static +FieldAccess AccessBuilder::ForFeedbackCellValue() { + FieldAccess access = {kTaggedBase, FeedbackCell::kValueOffset, + Handle(), MaybeHandle(), + Type::Any(), MachineType::TaggedPointer(), + kFullWriteBarrier}; + return access; +} + } // namespace compiler } // namespace internal } // namespace v8 diff --git a/src/compiler/access-builder.h b/src/compiler/access-builder.h index 7207e129b0..9edd3272a1 100644 --- a/src/compiler/access-builder.h +++ b/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); }; diff --git a/src/compiler/bytecode-graph-builder.cc b/src/compiler/bytecode-graph-builder.cc index 7200a467ab..f824aa3d3a 100644 --- a/src/compiler/bytecode-graph-builder.cc +++ b/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); diff --git a/src/compiler/graph-assembler.h b/src/compiler/graph-assembler.h index 16edacd516..9b0b5b42c1 100644 --- a/src/compiler/graph-assembler.h +++ b/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) { diff --git a/src/compiler/js-generic-lowering.cc b/src/compiler/js-generic-lowering.cc index d8d4644fcc..4cea874b00 100644 --- a/src/compiler/js-generic-lowering.cc +++ b/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); } } diff --git a/src/compiler/js-operator.cc b/src/compiler/js-operator.cc index a5f4c8b882..c71ff1fdd3 100644 --- a/src/compiler/js-operator.cc +++ b/src/compiler/js-operator.cc @@ -7,7 +7,6 @@ #include #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( \ - 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) diff --git a/src/compiler/js-operator.h b/src/compiler/js-operator.h index e5abe39d6f..05b7c7c8a7 100644 --- a/src/compiler/js-operator.h +++ b/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 { diff --git a/src/compiler/js-typed-lowering.cc b/src/compiler/js-typed-lowering.cc index 1bb4eff028..08160dfe43 100644 --- a/src/compiler/js-typed-lowering.cc +++ b/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); diff --git a/src/compiler/node.h b/src/compiler/node.h index 8072bab46e..add4116dac 100644 --- a/src/compiler/node.h +++ b/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;