From 2aa6751ef20eebeecce1e5fe9e0522d599453a17 Mon Sep 17 00:00:00 2001 From: Mythri A Date: Tue, 28 Apr 2020 13:37:21 +0100 Subject: [PATCH] [turboprop] Use feedback only for calls to builtins To reduce the number of deoptimizations in TurboProp use call feedback only when we know the call target is a builtin. Given that we don't inline in TurboProp, call feedback isn't really useful and using Generic lowering doesn't impact performance much. TurboProp still inlines builtins, so it is important to use this feedback for generating better optimized code. BUG: v8:10431 Change-Id: I24d51e43728f9aea3099767deb7800119fea40e2 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2116033 Commit-Queue: Mythri Alle Reviewed-by: Georg Neis Reviewed-by: Jakob Gruber Cr-Commit-Position: refs/heads/master@{#67468} --- src/compiler/js-call-reducer.cc | 40 +++++++++++++++++-- src/compiler/js-call-reducer.h | 4 +- src/compiler/js-type-hint-lowering.cc | 7 ++++ .../serializer-for-background-compilation.cc | 3 ++ test/cctest/cctest.status | 5 +++ test/mjsunit/mjsunit.status | 1 + 6 files changed, 55 insertions(+), 5 deletions(-) diff --git a/src/compiler/js-call-reducer.cc b/src/compiler/js-call-reducer.cc index dbf19bd00e..a2ce621e0f 100644 --- a/src/compiler/js-call-reducer.cc +++ b/src/compiler/js-call-reducer.cc @@ -3845,6 +3845,19 @@ bool ShouldUseCallICFeedback(Node* node) { } // namespace +bool JSCallReducer::IsBuiltinOrApiFunction(JSFunctionRef function) const { + if (should_disallow_heap_access() && !function.serialized()) { + TRACE_BROKER_MISSING(broker(), "data for function " << function); + return false; + } + + // TODO(neis): Add a way to check if function template info isn't serialized + // and add a warning in such cases. Currently we can't tell if function + // template info doesn't exist or wasn't serialized. + return function.shared().HasBuiltinId() || + function.shared().function_template_info().has_value(); +} + Reduction JSCallReducer::ReduceJSCall(Node* node) { DCHECK_EQ(IrOpcode::kJSCall, node->opcode()); CallParameters const& p = CallParametersOf(node->op()); @@ -3969,7 +3982,7 @@ Reduction JSCallReducer::ReduceJSCall(Node* node) { ProcessedFeedback const& feedback = broker()->GetFeedbackForCall(p.feedback()); if (feedback.IsInsufficient()) { - return ReduceSoftDeoptimize( + return ReduceForInsufficientFeedback( node, DeoptimizeReason::kInsufficientTypeFeedbackForCall); } @@ -3977,6 +3990,13 @@ Reduction JSCallReducer::ReduceJSCall(Node* node) { if (feedback_target.has_value() && feedback_target->map().is_callable()) { Node* target_function = jsgraph()->Constant(*feedback_target); + if (FLAG_turboprop) { + if (!feedback_target->IsJSFunction()) return NoChange(); + if (!IsBuiltinOrApiFunction(feedback_target->AsJSFunction())) { + return NoChange(); + } + } + // Check that the {target} is still the {target_function}. Node* check = graph()->NewNode(simplified()->ReferenceEqual(), target, target_function); @@ -4003,6 +4023,12 @@ Reduction JSCallReducer::ReduceJSCall(Node* node) { broker(), "feedback vector, not serialized: " << feedback_vector); return NoChange(); } + + if (FLAG_turboprop && + !feedback_vector.shared_function_info().HasBuiltinId()) { + return NoChange(); + } + Node* target_closure = effect = graph()->NewNode(simplified()->CheckClosure(feedback_cell.object()), target, effect, control); @@ -4412,7 +4438,7 @@ Reduction JSCallReducer::ReduceJSConstruct(Node* node) { ProcessedFeedback const& feedback = broker()->GetFeedbackForCall(p.feedback()); if (feedback.IsInsufficient()) { - return ReduceSoftDeoptimize( + return ReduceForInsufficientFeedback( node, DeoptimizeReason::kInsufficientTypeFeedbackForConstruct); } @@ -4823,9 +4849,15 @@ Reduction JSCallReducer::ReduceReturnReceiver(Node* node) { return Replace(receiver); } -Reduction JSCallReducer::ReduceSoftDeoptimize(Node* node, - DeoptimizeReason reason) { +Reduction JSCallReducer::ReduceForInsufficientFeedback( + Node* node, DeoptimizeReason reason) { + DCHECK(node->opcode() == IrOpcode::kJSCall || + node->opcode() == IrOpcode::kJSConstruct); if (!(flags() & kBailoutOnUninitialized)) return NoChange(); + // TODO(mythria): May be add additional flags to specify if we need to deopt + // on calls / construct rather than checking for TurboProp here. We may need + // it for NativeContextIndependent code too. + if (FLAG_turboprop) return NoChange(); Node* effect = NodeProperties::GetEffectInput(node); Node* control = NodeProperties::GetControlInput(node); diff --git a/src/compiler/js-call-reducer.h b/src/compiler/js-call-reducer.h index 9a37704d38..142e42789d 100644 --- a/src/compiler/js-call-reducer.h +++ b/src/compiler/js-call-reducer.h @@ -159,7 +159,7 @@ class V8_EXPORT_PRIVATE JSCallReducer final : public AdvancedReducer { const SharedFunctionInfoRef& shared); Reduction ReduceTypedArrayPrototypeToStringTag(Node* node); - Reduction ReduceSoftDeoptimize(Node* node, DeoptimizeReason reason); + Reduction ReduceForInsufficientFeedback(Node* node, DeoptimizeReason reason); Reduction ReduceMathUnary(Node* node, const Operator* op); Reduction ReduceMathBinary(Node* node, const Operator* op); @@ -218,6 +218,8 @@ class V8_EXPORT_PRIVATE JSCallReducer final : public AdvancedReducer { Node* control, Node** if_true, Node** if_false); Node* LoadReceiverElementsKind(Node* receiver, Node** effect, Node** control); + bool IsBuiltinOrApiFunction(JSFunctionRef target_ref) const; + Graph* graph() const; JSGraph* jsgraph() const { return jsgraph_; } JSHeapBroker* broker() const { return broker_; } diff --git a/src/compiler/js-type-hint-lowering.cc b/src/compiler/js-type-hint-lowering.cc index 5be5f2148a..5c9a287bcc 100644 --- a/src/compiler/js-type-hint-lowering.cc +++ b/src/compiler/js-type-hint-lowering.cc @@ -568,6 +568,13 @@ Node* JSTypeHintLowering::TryBuildSoftDeopt(FeedbackSlot slot, Node* effect, if (!(flags() & kBailoutOnUninitialized)) return nullptr; FeedbackSource source(feedback_vector(), slot); + // TODO(mythria): Think of adding flags to specify if we need a soft deopt for + // calls instead of using FLAG_turboprop here. + if (FLAG_turboprop && + broker()->GetFeedbackSlotKind(source) == FeedbackSlotKind::kCall) { + return nullptr; + } + if (!broker()->FeedbackIsInsufficient(source)) return nullptr; Node* deoptimize = jsgraph()->graph()->NewNode( diff --git a/src/compiler/serializer-for-background-compilation.cc b/src/compiler/serializer-for-background-compilation.cc index c6e352d90a..b7f0b68a0e 100644 --- a/src/compiler/serializer-for-background-compilation.cc +++ b/src/compiler/serializer-for-background-compilation.cc @@ -1090,6 +1090,9 @@ bool SerializerForBackgroundCompilation::BailoutOnUninitialized( // OSR entry point. TODO(neis): Support OSR? return false; } + if (FLAG_turboprop && feedback.slot_kind() == FeedbackSlotKind::kCall) { + return false; + } if (feedback.IsInsufficient()) { environment()->Kill(); return true; diff --git a/test/cctest/cctest.status b/test/cctest/cctest.status index 5e25e552fd..a45053d823 100644 --- a/test/cctest/cctest.status +++ b/test/cctest/cctest.status @@ -619,6 +619,11 @@ 'test-cpu-profiler/DeoptAtFirstLevelInlinedSource': [SKIP], 'test-cpu-profiler/DeoptAtSecondLevelInlinedSource': [SKIP], 'test-cpu-profiler/DeoptUntrackedFunction': [SKIP], + # Turboprop doesn't use call feedback and hence doesn't inline even if + # the inlining flag is explicitly set. + 'test-cpu-profiler/DetailedSourcePositionAPI_Inlining': [SKIP], + 'serializer-tester/BoundFunctionArguments': [SKIP], + 'serializer-tester/BoundFunctionTarget': [SKIP], }], # variant == turboprop ############################################################################## diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status index 6550c61b41..79c5da2ed8 100644 --- a/test/mjsunit/mjsunit.status +++ b/test/mjsunit/mjsunit.status @@ -1147,6 +1147,7 @@ 'compiler/opt-higher-order-functions': [SKIP], 'regress/regress-1049982-1': [SKIP], 'regress/regress-1049982-2': [SKIP], + 'es6/iterator-eager-deopt': [SKIP], # interrupt_budget overrides don't work with TurboProp. 'interrupt-budget-override': [SKIP],