From d65b3f4d3bd11c0d0d199a922d606108756629fd Mon Sep 17 00:00:00 2001 From: Hai Dang Date: Wed, 22 Aug 2018 13:57:38 +0200 Subject: [PATCH] Change IterableToList's check to a CSA call instead of a runtime call. The conditions checked by the CSA IsFastJSArrayWithNoCustomIteration is actually stronger than that of the runtime IterableToListCanBeElided. In particular, while IterableToListCanBeElided only checks that the prototype has no element when the array is holey, IsFastJSArrayWithNoCustomIteration always requires that the prototype has no element. Bug: v8:7980 Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng Change-Id: I28b086428d79682392413fb4182923184d7c1836 Reviewed-on: https://chromium-review.googlesource.com/1183671 Commit-Queue: Hai Dang Reviewed-by: Peter Marshall Reviewed-by: Georg Neis Cr-Commit-Position: refs/heads/master@{#55312} --- src/builtins/builtins-collections-gen.cc | 10 ++++------ src/builtins/builtins-iterator-gen.cc | 13 +++++++------ src/code-stub-assembler.cc | 3 +-- src/code-stub-assembler.h | 5 ++--- src/runtime/runtime-object.cc | 9 --------- src/runtime/runtime.h | 1 - 6 files changed, 14 insertions(+), 27 deletions(-) diff --git a/src/builtins/builtins-collections-gen.cc b/src/builtins/builtins-collections-gen.cc index 41961a6d8a..5808d2a98c 100644 --- a/src/builtins/builtins-collections-gen.cc +++ b/src/builtins/builtins-collections-gen.cc @@ -164,8 +164,7 @@ void BaseCollectionsAssembler::AddConstructorEntries( Variant variant, TNode context, TNode native_context, TNode collection, TNode initial_entries) { TVARIABLE(BoolT, use_fast_loop, - IsFastJSArrayWithNoCustomIteration(initial_entries, context, - native_context)); + IsFastJSArrayWithNoCustomIteration(initial_entries, context)); TNode at_least_space_for = EstimatedInitialSize(initial_entries, use_fast_loop.value()); Label allocate_table(this, &use_fast_loop), exit(this), fast_loop(this), @@ -186,8 +185,8 @@ void BaseCollectionsAssembler::AddConstructorEntries( TNode initial_entries_jsarray = UncheckedCast(initial_entries); #if DEBUG - CSA_ASSERT(this, IsFastJSArrayWithNoCustomIteration( - initial_entries_jsarray, context, native_context)); + CSA_ASSERT(this, IsFastJSArrayWithNoCustomIteration(initial_entries_jsarray, + context)); TNode original_initial_entries_map = LoadMap(initial_entries_jsarray); #endif @@ -228,8 +227,7 @@ void BaseCollectionsAssembler::AddConstructorEntriesFromFastJSArray( CSA_ASSERT( this, WordEqual(GetAddFunction(variant, native_context, collection), add_func)); - CSA_ASSERT(this, IsFastJSArrayWithNoCustomIteration(fast_jsarray, context, - native_context)); + CSA_ASSERT(this, IsFastJSArrayWithNoCustomIteration(fast_jsarray, context)); TNode length = SmiUntag(LoadFastJSArrayLength(fast_jsarray)); CSA_ASSERT(this, IntPtrGreaterThanOrEqual(length, IntPtrConstant(0))); CSA_ASSERT( diff --git a/src/builtins/builtins-iterator-gen.cc b/src/builtins/builtins-iterator-gen.cc index 8f3cf7e14a..bdb0ada730 100644 --- a/src/builtins/builtins-iterator-gen.cc +++ b/src/builtins/builtins-iterator-gen.cc @@ -201,13 +201,14 @@ TNode IteratorBuiltinsAssembler::IterableToList( TVARIABLE(JSArray, created_list); - // This is a fast-path for ignoring the iterator. - // TODO(petermarshall): Port IterableToListCanBeElided to CSA. - Node* elided = - CallRuntime(Runtime::kIterableToListCanBeElided, context, iterable); - CSA_ASSERT(this, IsBoolean(elided)); - Branch(IsTrue(elided), &fast_path, &slow_path); + // TODO(dhai): IsFastJSArrayWithNoCustomIteration unnecessarily checks that + // the prototype has no element even when the array is packed. + // Then the fast path will not be taken in the case when the array is packed + // and the prototype has some elements. + Branch(IsFastJSArrayWithNoCustomIteration(iterable, context), &fast_path, + &slow_path); + // This is a fast-path for ignoring the iterator. BIND(&fast_path); { TNode input_array = CAST(iterable); diff --git a/src/code-stub-assembler.cc b/src/code-stub-assembler.cc index bc117dd0f2..083b9bad73 100644 --- a/src/code-stub-assembler.cc +++ b/src/code-stub-assembler.cc @@ -1065,8 +1065,7 @@ TNode CodeStubAssembler::IsFastJSArray(SloppyTNode object, } TNode CodeStubAssembler::IsFastJSArrayWithNoCustomIteration( - TNode object, TNode context, - TNode native_context) { + TNode object, TNode context) { Label if_false(this, Label::kDeferred), if_fast(this), exit(this); TVARIABLE(BoolT, var_result); GotoIfForceSlowPath(&if_false); diff --git a/src/code-stub-assembler.h b/src/code-stub-assembler.h index 560c188722..b0b6100ba4 100644 --- a/src/code-stub-assembler.h +++ b/src/code-stub-assembler.h @@ -1732,9 +1732,8 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler { TNode IsExternalStringInstanceType(SloppyTNode instance_type); TNode IsFastJSArray(SloppyTNode object, SloppyTNode context); - TNode IsFastJSArrayWithNoCustomIteration( - TNode object, TNode context, - TNode native_context); + TNode IsFastJSArrayWithNoCustomIteration(TNode object, + TNode context); TNode IsFeedbackCell(SloppyTNode object); TNode IsFeedbackVector(SloppyTNode object); TNode IsContext(SloppyTNode object); diff --git a/src/runtime/runtime-object.cc b/src/runtime/runtime-object.cc index d8830a6655..88f419df82 100644 --- a/src/runtime/runtime-object.cc +++ b/src/runtime/runtime-object.cc @@ -1239,15 +1239,6 @@ RUNTIME_FUNCTION(Runtime_CreateDataProperty) { return *value; } -// Checks that 22.2.2.1.1 Runtime Semantics: IterableToList produces exactly the -// same result as doing nothing. -RUNTIME_FUNCTION(Runtime_IterableToListCanBeElided) { - HandleScope scope(isolate); - DCHECK_EQ(1, args.length()); - CONVERT_ARG_HANDLE_CHECKED(Object, obj, 0); - return isolate->heap()->ToBoolean(!obj->IterationHasObservableEffects()); -} - RUNTIME_FUNCTION(Runtime_GetOwnPropertyDescriptor) { HandleScope scope(isolate); diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index 57786b8941..ce10362b2a 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -334,7 +334,6 @@ namespace internal { F(HasProperty, 2, 1) \ F(InternalSetPrototype, 2, 1) \ F(IsJSReceiver, 1, 1) \ - F(IterableToListCanBeElided, 1, 1) \ F(KeyedGetProperty, 2, 1) \ F(NewObject, 2, 1) \ F(ObjectCreate, 2, 1) \