From d85c81b45aaec11aad2352d637755a6cb64ff9f9 Mon Sep 17 00:00:00 2001 From: Victor Gomes Date: Wed, 17 Feb 2021 11:25:58 +0100 Subject: [PATCH] [cleanup] Add UpdateFeedbackMode to CollectConstructFeedback to avoid test Change-Id: Icdd2d4a178415d240a82378ffa575e6e6b79dca1 Bug: v8:11429 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2697353 Reviewed-by: Leszek Swirski Reviewed-by: Nico Hartmann Reviewed-by: Toon Verwaest Commit-Queue: Victor Gomes Cr-Commit-Position: refs/heads/master@{#72805} --- src/builtins/base.tq | 4 ++++ src/builtins/builtins-constructor-gen.cc | 9 +++++---- src/builtins/ic-callable.tq | 25 ++++++++++++++++++++---- src/builtins/ic.tq | 7 ++++--- src/builtins/internal.tq | 1 - src/codegen/code-stub-assembler.h | 3 +++ src/interpreter/interpreter-assembler.cc | 4 ++-- 7 files changed, 39 insertions(+), 14 deletions(-) diff --git a/src/builtins/base.tq b/src/builtins/base.tq index 535df7b1a5..19059caf78 100644 --- a/src/builtins/base.tq +++ b/src/builtins/base.tq @@ -228,6 +228,10 @@ type Callable = JSFunction|JSBoundFunction|CallableJSProxy|CallableApiObject; type WriteBarrierMode generates 'TNode' constexpr 'WriteBarrierMode'; +extern enum UpdateFeedbackMode { kOptionalFeedback, kGuaranteedFeedback } +extern operator '==' macro UpdateFeedbackModeEqual( + constexpr UpdateFeedbackMode, constexpr UpdateFeedbackMode): constexpr bool; + extern enum UnicodeEncoding { UTF16, UTF32 } // Promise constants diff --git a/src/builtins/builtins-constructor-gen.cc b/src/builtins/builtins-constructor-gen.cc index 396cd4ac53..6808216f82 100644 --- a/src/builtins/builtins-constructor-gen.cc +++ b/src/builtins/builtins-constructor-gen.cc @@ -47,13 +47,12 @@ TF_BUILTIN(Construct_Baseline, CallOrConstructBuiltinsAssembler) { // TODO(verwaest): Only emit context loads where necessary auto context = LoadContextFromBaseline(); - // TODO(verwaest): Make sure CollectConstructFeedback knows we have a - // feedback vector. auto feedback_vector = LoadFeedbackVectorFromBaseline(); TVARIABLE(AllocationSite, allocation_site); Label if_construct_generic(this), if_construct_array(this); CollectConstructFeedback(context, target, new_target, feedback_vector, slot, + UpdateFeedbackMode::kGuaranteedFeedback, &if_construct_generic, &if_construct_array, &allocation_site); @@ -76,6 +75,7 @@ TF_BUILTIN(Construct_WithFeedback, CallOrConstructBuiltinsAssembler) { TVARIABLE(AllocationSite, allocation_site); Label if_construct_generic(this), if_construct_array(this); CollectConstructFeedback(context, target, new_target, feedback_vector, slot, + UpdateFeedbackMode::kOptionalFeedback, &if_construct_generic, &if_construct_array, &allocation_site); @@ -107,6 +107,7 @@ TF_BUILTIN(ConstructWithArrayLike_WithFeedback, TVARIABLE(AllocationSite, allocation_site); Label if_construct_generic(this), if_construct_array(this); CollectConstructFeedback(context, target, new_target, feedback_vector, slot, + UpdateFeedbackMode::kOptionalFeedback, &if_construct_generic, &if_construct_array, &allocation_site); @@ -137,13 +138,12 @@ TF_BUILTIN(ConstructWithSpread_Baseline, CallOrConstructBuiltinsAssembler) { // TODO(verwaest): Only emit context loads where necessary auto context = LoadContextFromBaseline(); - // TODO(verwaest): Make sure CollectConstructFeedback knows we have a - // feedback vector. auto feedback_vector = LoadFeedbackVectorFromBaseline(); TVARIABLE(AllocationSite, allocation_site); Label if_construct_generic(this), if_construct_array(this); CollectConstructFeedback(context, target, new_target, feedback_vector, slot, + UpdateFeedbackMode::kGuaranteedFeedback, &if_construct_generic, &if_construct_array, &allocation_site); @@ -167,6 +167,7 @@ TF_BUILTIN(ConstructWithSpread_WithFeedback, CallOrConstructBuiltinsAssembler) { TVARIABLE(AllocationSite, allocation_site); Label if_construct_generic(this), if_construct_array(this); CollectConstructFeedback(context, target, new_target, feedback_vector, slot, + UpdateFeedbackMode::kOptionalFeedback, &if_construct_generic, &if_construct_array, &allocation_site); diff --git a/src/builtins/ic-callable.tq b/src/builtins/ic-callable.tq index 8ee7fab17a..85525c4c68 100644 --- a/src/builtins/ic-callable.tq +++ b/src/builtins/ic-callable.tq @@ -141,10 +141,25 @@ macro BothTaggedEqualArrayFunction(implicit context: Context)( extern macro CreateAllocationSiteInFeedbackVector( FeedbackVector, uintptr): AllocationSite; +macro CastFeedbackVector( + maybeFeedbackVector: Undefined|FeedbackVector, + updateFeedbackMode: constexpr UpdateFeedbackMode): + FeedbackVector labels Fallback { + if constexpr (updateFeedbackMode == UpdateFeedbackMode::kGuaranteedFeedback) { + return UnsafeCast(maybeFeedbackVector); + } else if constexpr ( + updateFeedbackMode == UpdateFeedbackMode::kOptionalFeedback) { + return Cast(maybeFeedbackVector) otherwise goto Fallback; + } else { + unreachable; + } +} + macro CollectConstructFeedback(implicit context: Context)( target: JSAny, newTarget: JSAny, - maybeFeedbackVector: Undefined|FeedbackVector, - slotId: uintptr): never labels ConstructGeneric, + maybeFeedbackVector: Undefined|FeedbackVector, slotId: uintptr, + updateFeedbackMode: constexpr UpdateFeedbackMode): + never labels ConstructGeneric, ConstructArray(AllocationSite) { // TODO(v8:9891): Remove this assert once all callers are ported to Torque. // This assert ensures correctness of maybeFeedbackVector's type which can @@ -152,8 +167,10 @@ macro CollectConstructFeedback(implicit context: Context)( assert( IsUndefined(maybeFeedbackVector) || Is(maybeFeedbackVector)); - const feedbackVector = Cast(maybeFeedbackVector) - otherwise goto ConstructGeneric; + + const feedbackVector = CastFeedbackVector( + maybeFeedbackVector, updateFeedbackMode) otherwise goto ConstructGeneric; + IncrementCallCount(feedbackVector, slotId); try { diff --git a/src/builtins/ic.tq b/src/builtins/ic.tq index 848d7aad58..49d4e78fa5 100644 --- a/src/builtins/ic.tq +++ b/src/builtins/ic.tq @@ -25,11 +25,12 @@ macro CollectInstanceOfFeedback( @export macro CollectConstructFeedback(implicit context: Context)( target: JSAny, newTarget: JSAny, - maybeFeedbackVector: Undefined|FeedbackVector, - slotId: uintptr): never labels ConstructGeneric, + maybeFeedbackVector: Undefined|FeedbackVector, slotId: uintptr, + updateFeedbackMode: constexpr UpdateFeedbackMode): + never labels ConstructGeneric, ConstructArray(AllocationSite) { callable::CollectConstructFeedback( - target, newTarget, maybeFeedbackVector, slotId) + target, newTarget, maybeFeedbackVector, slotId, updateFeedbackMode) otherwise ConstructGeneric, ConstructArray; } diff --git a/src/builtins/internal.tq b/src/builtins/internal.tq index 596801e8e5..2c3348fb9f 100644 --- a/src/builtins/internal.tq +++ b/src/builtins/internal.tq @@ -50,7 +50,6 @@ builtin BytecodeBudgetInterruptFromCode(implicit context: Context)( extern transitioning builtin ForInFilter(implicit context: Context)( JSAny, HeapObject): JSAny; extern enum ForInFeedback extends uint31 { kAny, ...} -extern enum UpdateFeedbackMode { kOptionalFeedback, kGuaranteedFeedback } extern macro UpdateFeedback( SmiTagged, Undefined | FeedbackVector, uintptr, constexpr UpdateFeedbackMode); diff --git a/src/codegen/code-stub-assembler.h b/src/codegen/code-stub-assembler.h index f01e8f7cbb..85f11c24bc 100644 --- a/src/codegen/code-stub-assembler.h +++ b/src/codegen/code-stub-assembler.h @@ -3172,6 +3172,9 @@ class V8_EXPORT_PRIVATE CodeStubAssembler TNode closure); // Update the type feedback vector. + bool UpdateFeedbackModeEqual(UpdateFeedbackMode a, UpdateFeedbackMode b) { + return a == b; + } void UpdateFeedback(TNode feedback, TNode maybe_feedback_vector, TNode slot_id, UpdateFeedbackMode mode); diff --git a/src/interpreter/interpreter-assembler.cc b/src/interpreter/interpreter-assembler.cc index a873f817e0..c28215e379 100644 --- a/src/interpreter/interpreter-assembler.cc +++ b/src/interpreter/interpreter-assembler.cc @@ -802,8 +802,8 @@ TNode InterpreterAssembler::Construct( construct_array(this, &var_site); CollectConstructFeedback(context, target, new_target, maybe_feedback_vector, - slot_id, &construct_generic, &construct_array, - &var_site); + slot_id, UpdateFeedbackMode::kOptionalFeedback, + &construct_generic, &construct_array, &var_site); BIND(&construct_generic); {