From d6465298b6777b57f2b1d152d5f6d61729b55862 Mon Sep 17 00:00:00 2001 From: Mike Stanton Date: Fri, 8 Mar 2019 11:40:11 +0100 Subject: [PATCH] [Torque] Port Array.prototype.shift to Torque Optimizations to use fast memmove to move elements are preserved, as well as heuristics for bailout to the runtime if left or right trimming is desired. Bug: v8:7672 Change-Id: I01ffc1143b63d705d99a40eab3a7e873596d0aa4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1499495 Commit-Queue: Michael Stanton Reviewed-by: Tobias Tebbi Cr-Commit-Position: refs/heads/master@{#60118} --- BUILD.gn | 2 + src/builtins/array-shift.tq | 113 +++++++++++++++++++++++ src/builtins/base.tq | 36 +++++++- src/builtins/builtins-array-gen.cc | 133 ---------------------------- src/builtins/builtins-definitions.h | 1 - 5 files changed, 149 insertions(+), 136 deletions(-) create mode 100644 src/builtins/array-shift.tq diff --git a/BUILD.gn b/BUILD.gn index 854155df8b..afb5564519 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -913,6 +913,7 @@ torque_files = [ "src/builtins/array-reduce.tq", "src/builtins/array-reduce-right.tq", "src/builtins/array-reverse.tq", + "src/builtins/array-shift.tq", "src/builtins/array-slice.tq", "src/builtins/array-some.tq", "src/builtins/array-splice.tq", @@ -948,6 +949,7 @@ torque_namespaces = [ "array-map", "array-of", "array-reverse", + "array-shift", "array-slice", "array-splice", "array-unshift", diff --git a/src/builtins/array-shift.tq b/src/builtins/array-shift.tq new file mode 100644 index 0000000000..ad219672b4 --- /dev/null +++ b/src/builtins/array-shift.tq @@ -0,0 +1,113 @@ +// Copyright 2019 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +namespace array_shift { + extern builtin ArrayShift(Context, JSFunction, Object, int32); + + macro TryFastArrayShift(implicit context: Context)( + receiver: Object, arguments: constexpr Arguments): Object + labels Slow { + const array: FastJSArray = Cast(receiver) otherwise Slow; + let witness = NewFastJSArrayWitness(array); + + witness.EnsureArrayPushable() otherwise Slow; + + if (array.length == 0) { + return Undefined; + } + + try { + const newLength = array.length - 1; + + // Check that we're not supposed to right-trim the backing store, as + // implemented in elements.cc:ElementsAccessorBase::SetLengthImpl. + if ((newLength + newLength + kMinAddedElementsCapacity) < + array.elements.length) { + goto Runtime; + } + + // Check that we're not supposed to left-trim the backing store, as + // implemented in elements.cc:FastElementsAccessor::MoveElements. + if (newLength > kMaxCopyElements) goto Runtime; + + const result = witness.LoadElementOrUndefined(0); + witness.ChangeLength(newLength); + witness.MoveElements(0, 1, Convert(newLength)); + return result; + } + label Runtime { + tail ArrayShift( + context, LoadTargetFromFrame(), Undefined, + Convert(arguments.length)); + } + } + + transitioning macro GenericArrayShift(implicit context: + Context)(receiver: Object): Object { + // 1. Let O be ? ToObject(this value). + const object: JSReceiver = ToObject_Inline(context, receiver); + + // 2. Let len be ? ToLength(? Get(O, "length")). + const length: Number = GetLengthProperty(object); + + // 3. If len is zero, then + if (length == 0) { + // a. Perform ? Set(O, "length", 0, true). + SetProperty(object, kLengthString, Convert(0)); + // b. Return undefined. + return Undefined; + } + + // 4. Let first be ? Get(O, "0"). + const first = GetProperty(object, Convert(0)); + // 5. Let k be 1. + let k: Number = 1; + // 6. Repeat, while k < len + while (k < length) { + // a. Let from be ! ToString(k). + const from: Number = k; + + // b. Let to be ! ToString(k - 1). + const to: Number = k - 1; + + // c. Let fromPresent be ? HasProperty(O, from). + const fromPresent: Boolean = HasProperty(object, from); + + // d. If fromPresent is true, then + if (fromPresent == True) { + // i. Let fromVal be ? Get(O, from). + const fromValue: Object = GetProperty(object, from); + + // ii. Perform ? Set(O, to, fromValue, true). + SetProperty(object, to, fromValue); + } else { + // i. Perform ? DeletePropertyOrThrow(O, to). + DeleteProperty(object, to, kStrict); + } + + // f. Increase k by 1. + k++; + } + + // 7. Perform ? DeletePropertyOrThrow(O, ! ToString(len - 1)). + DeleteProperty(object, length - 1, kStrict); + + // 8. Perform ? Set(O, "length", len - 1, true). + SetProperty(object, kLengthString, length - 1); + + // 9. Return first. + return first; + } + + // https://tc39.github.io/ecma262/#sec-array.prototype.shift + transitioning javascript builtin ArrayPrototypeShift( + implicit context: Context)(receiver: Object, ...arguments): Object { + try { + return TryFastArrayShift(receiver, arguments) otherwise Slow; + } + label Slow { + return GenericArrayShift(receiver); + } + } +} diff --git a/src/builtins/base.tq b/src/builtins/base.tq index 628a65f41c..8998afb494 100644 --- a/src/builtins/base.tq +++ b/src/builtins/base.tq @@ -536,7 +536,10 @@ const kFixedTypedArrayBaseHeaderSize: constexpr intptr generates 'FixedTypedArrayBase::kHeaderSize'; const kObjectAlignmentMask: constexpr intptr generates 'kObjectAlignmentMask'; - +const kMinAddedElementsCapacity: + constexpr int31 generates 'JSObject::kMinAddedElementsCapacity'; +const kMaxCopyElements: + constexpr int31 generates 'JSArray::kMaxCopyElements'; const kMaxRegularHeapObjectSize: constexpr int31 generates 'kMaxRegularHeapObjectSize'; @@ -1654,6 +1657,11 @@ extern builtin ExtractFastJSArray(Context, JSArray, Smi, Smi): JSArray; extern macro MoveElements( constexpr ElementsKind, FixedArrayBase, intptr, intptr, intptr): void; +macro TorqueMoveElementsSmi( + elements: FixedArray, dstIndex: intptr, srcIndex: intptr, + count: intptr): void { + MoveElements(HOLEY_SMI_ELEMENTS, elements, dstIndex, srcIndex, count); +} macro TorqueMoveElements( elements: FixedArray, dstIndex: intptr, srcIndex: intptr, count: intptr): void { @@ -1753,11 +1761,17 @@ struct FastJSArrayWitness { } } - EnsureArrayPushable() labels Failed { + EnsureArrayPushable(implicit context: Context)() labels Failed { EnsureArrayPushable(this.map) otherwise Failed; + array::EnsureWriteableFastElements(this.unstable); this.arrayIsPushable = true; } + ChangeLength(newLength: Smi) { + assert(this.arrayIsPushable); + this.unstable.length = newLength; + } + Push(value: Object) labels Failed { assert(this.arrayIsPushable); if (this.hasDoubles) { @@ -1775,6 +1789,24 @@ struct FastJSArrayWitness { } } + MoveElements(dst: intptr, src: intptr, length: intptr) { + assert(this.arrayIsPushable); + if (this.hasDoubles) { + let elements: FixedDoubleArray = + Cast(this.unstable.elements) + otherwise unreachable; + TorqueMoveElements(elements, dst, src, length); + } else { + let elements: FixedArray = Cast(this.unstable.elements) + otherwise unreachable; + if (this.hasSmis) { + TorqueMoveElementsSmi(elements, dst, src, length); + } else { + TorqueMoveElements(elements, dst, src, length); + } + } + } + stable: JSArray; unstable: FastJSArray; map: Map; diff --git a/src/builtins/builtins-array-gen.cc b/src/builtins/builtins-array-gen.cc index 7dfe705811..fe78403d52 100644 --- a/src/builtins/builtins-array-gen.cc +++ b/src/builtins/builtins-array-gen.cc @@ -592,139 +592,6 @@ TF_BUILTIN(ArrayPrototypePush, CodeStubAssembler) { } } -TF_BUILTIN(ArrayPrototypeShift, CodeStubAssembler) { - TNode argc = - UncheckedCast(Parameter(Descriptor::kJSActualArgumentsCount)); - TNode context = CAST(Parameter(Descriptor::kContext)); - CSA_ASSERT(this, IsUndefined(Parameter(Descriptor::kJSNewTarget))); - - CodeStubArguments args(this, ChangeInt32ToIntPtr(argc)); - TNode receiver = args.GetReceiver(); - - Label runtime(this, Label::kDeferred); - Label fast(this); - - // Only shift in this stub if - // 1) the array has fast elements - // 2) the length is writable, - // 3) the elements backing store isn't copy-on-write, - // 4) we aren't supposed to shrink the backing store, - // 5) we aren't supposed to left-trim the backing store. - - // 1) Check that the array has fast elements. - BranchIfFastJSArray(receiver, context, &fast, &runtime); - - BIND(&fast); - { - TNode array_receiver = CAST(receiver); - CSA_ASSERT(this, TaggedIsPositiveSmi(LoadJSArrayLength(array_receiver))); - - // 2) Ensure that the length is writable. - // This check needs to happen before the check for length zero. - // The spec requires a "SetProperty(array, 'length', 0)" call when - // the length is zero. This must throw an exception in the case of a - // read-only length. - EnsureArrayLengthWritable(LoadMap(array_receiver), &runtime); - - TNode length = - LoadAndUntagObjectField(array_receiver, JSArray::kLengthOffset); - Label return_undefined(this), fast_elements_tagged(this), - fast_elements_smi(this); - GotoIf(IntPtrEqual(length, IntPtrConstant(0)), &return_undefined); - - // 3) Check that the elements backing store isn't copy-on-write. - TNode elements = LoadElements(array_receiver); - GotoIf(WordEqual(LoadMap(elements), LoadRoot(RootIndex::kFixedCOWArrayMap)), - &runtime); - - TNode new_length = IntPtrSub(length, IntPtrConstant(1)); - - // 4) Check that we're not supposed to right-trim the backing store, as - // implemented in elements.cc:ElementsAccessorBase::SetLengthImpl. - Node* capacity = SmiUntag(LoadFixedArrayBaseLength(elements)); - GotoIf(IntPtrLessThan( - IntPtrAdd(IntPtrAdd(new_length, new_length), - IntPtrConstant(JSObject::kMinAddedElementsCapacity)), - capacity), - &runtime); - - // 5) Check that we're not supposed to left-trim the backing store, as - // implemented in elements.cc:FastElementsAccessor::MoveElements. - GotoIf(IntPtrGreaterThan(new_length, - IntPtrConstant(JSArray::kMaxCopyElements)), - &runtime); - - StoreObjectFieldNoWriteBarrier(array_receiver, JSArray::kLengthOffset, - SmiTag(new_length)); - - TNode element_zero = IntPtrConstant(0); - TNode element_one = IntPtrConstant(1); - TNode elements_kind = LoadElementsKind(array_receiver); - GotoIf( - Int32LessThanOrEqual(elements_kind, Int32Constant(HOLEY_SMI_ELEMENTS)), - &fast_elements_smi); - GotoIf(Int32LessThanOrEqual(elements_kind, Int32Constant(HOLEY_ELEMENTS)), - &fast_elements_tagged); - - // Fast double elements kind: - { - CSA_ASSERT(this, - Int32LessThanOrEqual(elements_kind, - Int32Constant(HOLEY_DOUBLE_ELEMENTS))); - - VARIABLE(result, MachineRepresentation::kTagged, UndefinedConstant()); - - Label move_elements(this); - result.Bind(AllocateHeapNumberWithValue(LoadFixedDoubleArrayElement( - CAST(elements), element_zero, &move_elements))); - Goto(&move_elements); - BIND(&move_elements); - - MoveElements(HOLEY_DOUBLE_ELEMENTS, elements, element_zero, element_one, - new_length); - StoreFixedDoubleArrayHole(CAST(elements), new_length); - args.PopAndReturn(result.value()); - } - - BIND(&fast_elements_tagged); - { - TNode elements_fixed_array = CAST(elements); - Node* value = LoadFixedArrayElement(elements_fixed_array, 0); - MoveElements(HOLEY_ELEMENTS, elements, element_zero, element_one, - new_length); - StoreFixedArrayElement(elements_fixed_array, new_length, - TheHoleConstant()); - GotoIf(WordEqual(value, TheHoleConstant()), &return_undefined); - args.PopAndReturn(value); - } - - BIND(&fast_elements_smi); - { - TNode elements_fixed_array = CAST(elements); - Node* value = LoadFixedArrayElement(elements_fixed_array, 0); - MoveElements(HOLEY_SMI_ELEMENTS, elements, element_zero, element_one, - new_length); - StoreFixedArrayElement(elements_fixed_array, new_length, - TheHoleConstant()); - GotoIf(WordEqual(value, TheHoleConstant()), &return_undefined); - args.PopAndReturn(value); - } - - BIND(&return_undefined); - { args.PopAndReturn(UndefinedConstant()); } - } - - BIND(&runtime); - { - // We are not using Parameter(Descriptor::kJSTarget) and loading the value - // from the current frame here in order to reduce register pressure on the - // fast path. - TNode target = LoadTargetFromFrame(); - TailCallBuiltin(Builtins::kArrayShift, context, target, UndefinedConstant(), - argc); - } -} - TF_BUILTIN(ExtractFastJSArray, ArrayBuiltinsAssembler) { ParameterMode mode = OptimalParameterMode(); TNode context = CAST(Parameter(Descriptor::kContext)); diff --git a/src/builtins/builtins-definitions.h b/src/builtins/builtins-definitions.h index fcff4f922f..425fcdcd48 100644 --- a/src/builtins/builtins-definitions.h +++ b/src/builtins/builtins-definitions.h @@ -346,7 +346,6 @@ namespace internal { TFJ(ArrayPrototypePush, SharedFunctionInfo::kDontAdaptArgumentsSentinel) \ /* ES6 #sec-array.prototype.shift */ \ CPP(ArrayShift) \ - TFJ(ArrayPrototypeShift, SharedFunctionInfo::kDontAdaptArgumentsSentinel) \ /* ES6 #sec-array.prototype.unshift */ \ CPP(ArrayUnshift) \ /* Support for Array.from and other array-copying idioms */ \