From ced2e4eec541b04908ea73755e9129ea2167fea4 Mon Sep 17 00:00:00 2001 From: Jakob Gruber Date: Mon, 18 Feb 2019 08:18:11 +0000 Subject: [PATCH] Revert "[builtins]: Optimize CreateTypedArray to use element size log 2 for calculations." MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit c9ef0405c786c86f5ad9755b9b3573d2a43d9757. Reason for revert: https://crbug.com/932034 Original change's description: > [builtins]: Optimize CreateTypedArray to use element size log 2 for calculations. > > TypedArrayElementsInfo now represents an element's size as a log 2 and typed as > uintptr. This simplifies and speeds up (avoids possible HeapNumber allocations) a > number of calculations: > > - Number of Elements (length) -> Byte Length - is now a WordShl > - Byte Length -> Number of Elements (length) - is now a WordShr > - Testing alignment (byte offset or length) - is now a WordAnd > > These element/byte length related calculations are encapsulated in > TypedArrayElementsInfo as struct methods. > > This reduces the size of CreateTypedArray by 2.125 KB (24%) on Mac x64.release: > - Before: 9,088 > - After: 6,896 > > This improves the performance of the following microbencmarks > - TypedArrays-ConstructWithBuffer: ~87% > - TypedArrays-SubarrayNoSpecies: ~28% > > Bug: v8:7161 > Change-Id: I2239fd0e0af9d3ad55cd52318088d3c7c913ae44 > Reviewed-on: https://chromium-review.googlesource.com/c/1456299 > Commit-Queue: Peter Wong > Reviewed-by: Jakob Gruber > Reviewed-by: Simon Zünd > Cr-Commit-Position: refs/heads/master@{#59531} TBR=peter.wm.wong@gmail.com,jgruber@chromium.org,petermarshall@chromium.org,szuend@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: v8:7161, chromium:932034 Change-Id: I3da95447ce34f84d01629d2791868f3adcdfb387 Reviewed-on: https://chromium-review.googlesource.com/c/1475764 Commit-Queue: Jakob Gruber Reviewed-by: Jakob Gruber Cr-Commit-Position: refs/heads/master@{#59645} --- src/builtins/base.tq | 14 ++-- src/builtins/builtins-typed-array-gen.cc | 59 +++++++-------- src/builtins/builtins-typed-array-gen.h | 6 +- src/builtins/typed-array-createtypedarray.tq | 76 +++++++++++++------- src/builtins/typed-array.tq | 31 +------- src/code-stub-assembler.cc | 16 ++--- src/code-stub-assembler.h | 5 +- src/external-reference.cc | 5 -- src/external-reference.h | 2 - 9 files changed, 96 insertions(+), 118 deletions(-) diff --git a/src/builtins/base.tq b/src/builtins/base.tq index a47f5e79cd..b26e0463a1 100644 --- a/src/builtins/base.tq +++ b/src/builtins/base.tq @@ -340,7 +340,6 @@ const kTypedArrayMaxByteLength: const V8_TYPED_ARRAY_MAX_SIZE_IN_HEAP: constexpr int31 generates 'V8_TYPED_ARRAY_MAX_SIZE_IN_HEAP'; const kMaxSafeInteger: constexpr float64 generates 'kMaxSafeInteger'; -const kSmiMaxValue: constexpr uintptr generates 'kSmiMaxValue'; const kStringMaxLength: constexpr int31 generates 'String::kMaxLength'; const kFixedArrayMaxLength: constexpr int31 generates 'FixedArray::kMaxLength'; @@ -594,6 +593,10 @@ extern operator '>=' macro BranchIfNumberGreaterThanOrEqual( Number, Number): never labels Taken, NotTaken; +extern builtin Divide(implicit context: Context)(Object, Object): Numeric; +extern builtin Modulus(implicit context: Context)(Object, Object): Numeric; +extern builtin Subtract(implicit context: Context)(Object, Object): Numeric; + // The type of all tagged values that can safely be compared with WordEqual. type TaggedWithIdentity = JSReceiver | FixedArrayBase | Oddball | Map | EmptyString; @@ -633,7 +636,6 @@ extern operator '|' macro WordOr(intptr, intptr): intptr; extern operator '+' macro UintPtrAdd(uintptr, uintptr): uintptr; extern operator '-' macro UintPtrSub(uintptr, uintptr): uintptr; -extern operator '<<' macro WordShl(uintptr, uintptr): uintptr; extern operator '>>>' macro WordShr(uintptr, uintptr): uintptr; extern operator '&' macro WordAnd(uintptr, uintptr): uintptr; extern operator '|' macro WordOr(uintptr, uintptr): uintptr; @@ -985,11 +987,6 @@ extern macro LoadNativeContext(Context): NativeContext; extern macro LoadJSArrayElementsMap(constexpr ElementsKind, Context): Map; extern macro LoadJSArrayElementsMap(ElementsKind, Context): Map; extern macro ChangeNonnegativeNumberToUintPtr(Number): uintptr; -extern macro TryNumberToUintPtr(Number): uintptr labels IfNegative; -macro TryUintPtrToPositiveSmi(ui: uintptr): PositiveSmi labels IfOverflow { - if (ui > kSmiMaxValue) goto IfOverflow; - return %RawDownCast(SmiTag(Signed(ui))); -} extern macro NumberConstant(constexpr float64): Number; extern macro NumberConstant(constexpr int32): Number; @@ -1144,9 +1141,6 @@ Convert(ui: uintptr): uint32 { Convert(s: Smi): intptr { return SmiUntag(s); } -Convert(ps: PositiveSmi): uintptr { - return Unsigned(SmiUntag(ps)); -} Convert(ui: uintptr): intptr { const i = Signed(ui); assert(i >= 0); diff --git a/src/builtins/builtins-typed-array-gen.cc b/src/builtins/builtins-typed-array-gen.cc index 9888911c84..0ff091fa96 100644 --- a/src/builtins/builtins-typed-array-gen.cc +++ b/src/builtins/builtins-typed-array-gen.cc @@ -44,15 +44,6 @@ TNode TypedArrayBuiltinsAssembler::LoadMapForType( return var_typed_map.value(); } -TNode TypedArrayBuiltinsAssembler::IsMockArrayBufferAllocatorFlag() { - TNode flag_value = UncheckedCast(Load( - MachineType::Uint8(), - ExternalConstant( - ExternalReference::address_of_mock_arraybuffer_allocator_flag()))); - return Word32NotEqual(Word32And(flag_value, Int32Constant(0xFF)), - Int32Constant(0)); -} - // The byte_offset can be higher than Smi range, in which case to perform the // pointer arithmetic necessary to calculate external_pointer, converting // byte_offset to an intptr is more difficult. The max byte_offset is 8 * MaxSmi @@ -61,22 +52,9 @@ TNode TypedArrayBuiltinsAssembler::IsMockArrayBufferAllocatorFlag() { // bit platforms could theoretically have an offset up to 2^35 - 1, so we may // need to convert the float heap number to an intptr. TNode TypedArrayBuiltinsAssembler::CalculateExternalPointer( - TNode backing_store, TNode byte_offset) { - TNode external_pointer = UintPtrAdd(backing_store, byte_offset); - -#ifdef DEBUG - // Assert no overflow has occurred. Only assert if the mock array buffer - // allocator is NOT used. When the mock array buffer is used, impossibly - // large allocations are allowed that would erroneously cause an overflow and - // this assertion to fail. - Label next(this); - GotoIf(IsMockArrayBufferAllocatorFlag(), &next); - CSA_ASSERT(this, UintPtrGreaterThanOrEqual(external_pointer, backing_store)); - Goto(&next); - BIND(&next); -#endif // DEBUG - - return external_pointer; + TNode backing_store, TNode byte_offset) { + return Unsigned( + IntPtrAdd(backing_store, ChangeNonnegativeNumberToUintPtr(byte_offset))); } // Setup the TypedArray which is under construction. @@ -107,7 +85,7 @@ void TypedArrayBuiltinsAssembler::AttachBuffer(TNode holder, TNode buffer, TNode map, TNode length, - TNode byte_offset) { + TNode byte_offset) { CSA_ASSERT(this, TaggedIsPositiveSmi(length)); StoreObjectField(holder, JSArrayBufferView::kBufferOffset, buffer); @@ -217,6 +195,29 @@ Node* TypedArrayBuiltinsAssembler::LoadDataPtr(Node* typed_array) { return LoadFixedTypedArrayBackingStore(CAST(elements)); } +TNode TypedArrayBuiltinsAssembler::ByteLengthIsValid( + TNode byte_length) { + Label smi(this), done(this); + TVARIABLE(BoolT, is_valid); + GotoIf(TaggedIsSmi(byte_length), &smi); + + TNode float_value = LoadHeapNumberValue(CAST(byte_length)); + TNode max_byte_length_double = + Float64Constant(FixedTypedArrayBase::kMaxByteLength); + is_valid = Float64LessThanOrEqual(float_value, max_byte_length_double); + Goto(&done); + + BIND(&smi); + TNode max_byte_length = + IntPtrConstant(FixedTypedArrayBase::kMaxByteLength); + is_valid = + UintPtrLessThanOrEqual(SmiUntag(CAST(byte_length)), max_byte_length); + Goto(&done); + + BIND(&done); + return is_valid.value(); +} + TF_BUILTIN(TypedArrayBaseConstructor, TypedArrayBuiltinsAssembler) { TNode context = CAST(Parameter(Descriptor::kContext)); ThrowTypeError(context, MessageTemplate::kConstructAbstractClass, @@ -336,7 +337,7 @@ TypedArrayBuiltinsFromDSLAssembler::TypedArrayElementsInfo TypedArrayBuiltinsAssembler::GetTypedArrayElementsInfo( TNode typed_array) { TNode elements_kind = LoadElementsKind(typed_array); - TVARIABLE(UintPtrT, var_size_log2); + TVARIABLE(Smi, var_element_size); TVARIABLE(Map, var_map); ReadOnlyRoots roots(isolate()); @@ -344,14 +345,14 @@ TypedArrayBuiltinsAssembler::GetTypedArrayElementsInfo( elements_kind, [&](ElementsKind kind, int size, int typed_array_fun_index) { DCHECK_GT(size, 0); - var_size_log2 = UintPtrConstant(ElementsKindToShiftSize(kind)); + var_element_size = SmiConstant(size); Handle map(roots.MapForFixedTypedArray(kind), isolate()); var_map = HeapConstant(map); }); return TypedArrayBuiltinsFromDSLAssembler::TypedArrayElementsInfo{ - var_size_log2.value(), var_map.value(), elements_kind}; + var_element_size.value(), var_map.value(), elements_kind}; } TNode TypedArrayBuiltinsAssembler::GetDefaultConstructor( diff --git a/src/builtins/builtins-typed-array-gen.h b/src/builtins/builtins-typed-array-gen.h index 9edf2412af..40f5bb0c9f 100644 --- a/src/builtins/builtins-typed-array-gen.h +++ b/src/builtins/builtins-typed-array-gen.h @@ -38,7 +38,7 @@ class TypedArrayBuiltinsAssembler : public CodeStubAssembler { TNode byte_length); void AttachBuffer(TNode holder, TNode buffer, TNode map, TNode length, - TNode byte_offset); + TNode byte_offset); TNode AllocateEmptyOnHeapBuffer(TNode context, TNode holder, @@ -49,10 +49,10 @@ class TypedArrayBuiltinsAssembler : public CodeStubAssembler { TNode length); TNode LoadMapForType(TNode array); - TNode IsMockArrayBufferAllocatorFlag(); TNode CalculateExternalPointer(TNode backing_store, - TNode byte_offset); + TNode byte_offset); Node* LoadDataPtr(Node* typed_array); + TNode ByteLengthIsValid(TNode byte_length); // Returns true if kind is either UINT8_ELEMENTS or UINT8_CLAMPED_ELEMENTS. TNode IsUint8ElementsKind(TNode kind); diff --git a/src/builtins/typed-array-createtypedarray.tq b/src/builtins/typed-array-createtypedarray.tq index 043ead7f1b..0a8d93a651 100644 --- a/src/builtins/typed-array-createtypedarray.tq +++ b/src/builtins/typed-array-createtypedarray.tq @@ -14,12 +14,15 @@ namespace typed_array_createtypedarray { implicit context: Context)(JSTypedArray, uintptr): JSArrayBuffer; extern macro TypedArrayBuiltinsAssembler::AllocateOnHeapElements( Map, intptr, Number): FixedTypedArrayBase; + extern macro TypedArrayBuiltinsAssembler::ByteLengthIsValid(Number): bool; + extern macro TypedArrayBuiltinsAssembler::GetTypedArrayElementsInfo( + ElementsKind): typed_array::TypedArrayElementsInfo; extern macro TypedArrayBuiltinsAssembler::IsSharedArrayBuffer(JSArrayBuffer): bool; extern macro TypedArrayBuiltinsAssembler::SetupTypedArray( JSTypedArray, Smi, uintptr, uintptr): void; extern macro TypedArrayBuiltinsAssembler::AttachBuffer( - JSTypedArray, JSArrayBuffer, Map, Smi, uintptr): void; + JSTypedArray, JSArrayBuffer, Map, Smi, Number): void; extern runtime ThrowInvalidTypedArrayAlignment(implicit context: Context)( Map, String): never; @@ -36,8 +39,10 @@ namespace typed_array_createtypedarray { initialize: constexpr bool, typedArray: JSTypedArray, length: PositiveSmi, elementsInfo: typed_array::TypedArrayElementsInfo, bufferConstructor: JSReceiver): Object { - const byteLength = elementsInfo.CalculateByteLength(length); - const byteLengthNum = Convert(byteLength); + assert(Is(length)); + + const byteLengthNum = SmiMul(length, elementsInfo.size); + const byteLength = Convert(byteLengthNum); const defaultConstructor = GetArrayBufferFunction(); try { @@ -71,7 +76,7 @@ namespace typed_array_createtypedarray { } label AttachBuffer(bufferObj: Object) { const buffer = Cast(bufferObj) otherwise unreachable; - const byteOffset: uintptr = 0; + const byteOffset: Smi = 0; AttachBuffer(typedArray, buffer, elementsInfo.map, length, byteOffset); } @@ -81,6 +86,18 @@ namespace typed_array_createtypedarray { return Undefined; } + macro TypedArrayInitializeWithBuffer( + typedArray: JSTypedArray, length: PositiveSmi, buffer: JSArrayBuffer, + elementsInfo: typed_array::TypedArrayElementsInfo, byteOffset: Number) { + const byteLength: Number = SmiMul(length, elementsInfo.size); + + SetupTypedArray( + typedArray, length, Convert(byteOffset), + Convert(byteLength)); + + AttachBuffer(typedArray, buffer, elementsInfo.map, length, byteOffset); + } + // 22.2.4.2 TypedArray ( length ) // ES #sec-typedarray-length transitioning macro ConstructByLength(implicit context: Context)( @@ -124,9 +141,10 @@ namespace typed_array_createtypedarray { goto IfSlow; } else if (length > 0) { - const byteLength = elementsInfo.CalculateByteLength(length); - assert(byteLength <= kTypedArrayMaxByteLength); - typed_array::CallCMemcpy(typedArray.data_ptr, src.data_ptr, byteLength); + const byteLength: Number = SmiMul(length, elementsInfo.size); + assert(ByteLengthIsValid(byteLength)); + typed_array::CallCMemcpy( + typedArray.data_ptr, src.data_ptr, Convert(byteLength)); } } label IfSlow deferred { @@ -169,27 +187,36 @@ namespace typed_array_createtypedarray { goto IfConstructByArrayLike(srcTypedArray, length, bufferConstructor); } + // Determines if `bytes` (byte offset or length) cannot be evenly divded by + // element size. + macro IsUnaligned(implicit context: Context)(bytes: Number, elementSize: Smi): + bool { + const kZero: Smi = 0; + if (bytes == kZero) return false; + const remainder: Number = + Cast(Modulus(bytes, elementSize)) otherwise unreachable; + return remainder != kZero; + } + // 22.2.4.5 TypedArray ( buffer, byteOffset, length ) // ES #sec-typedarray-buffer-byteoffset-length macro ConstructByArrayBuffer(implicit context: Context)( typedArray: JSTypedArray, buffer: JSArrayBuffer, byteOffset: Object, length: Object, elementsInfo: typed_array::TypedArrayElementsInfo): void { try { - let offset: uintptr = 0; + let offset: Number = FromConstexpr(0); if (byteOffset != Undefined) { // 6. Let offset be ? ToIndex(byteOffset). - offset = TryNumberToUintPtr( - ToInteger_Inline(context, byteOffset, kTruncateMinusZero)) - otherwise goto IfInvalidOffset; + offset = ToInteger_Inline(context, byteOffset, kTruncateMinusZero); + if (offset < 0) goto IfInvalidOffset; // 7. If offset modulo elementSize ≠ 0, throw a RangeError exception. - if (elementsInfo.IsUnaligned(offset)) { + if (IsUnaligned(offset, elementsInfo.size)) { goto IfInvalidAlignment('start offset'); } } let newLength: PositiveSmi = 0; - let newByteLength: uintptr; // 8. If length is present and length is not undefined, then if (length != Undefined) { // a. Let newLength be ? ToIndex(length). @@ -202,13 +229,13 @@ namespace typed_array_createtypedarray { } // 10. Let bufferByteLength be buffer.[[ArrayBufferByteLength]]. - const bufferByteLength: uintptr = buffer.byte_length; + const bufferByteLength: Number = Convert(buffer.byte_length); // 11. If length is either not present or undefined, then if (length == Undefined) { // a. If bufferByteLength modulo elementSize ≠ 0, throw a RangeError // exception. - if (elementsInfo.IsUnaligned(bufferByteLength)) { + if (IsUnaligned(bufferByteLength, elementsInfo.size)) { goto IfInvalidAlignment('byte length'); } @@ -218,24 +245,25 @@ namespace typed_array_createtypedarray { // Spec step 16 length calculated here to avoid recalculating the length // in the step 12 branch. - newByteLength = bufferByteLength - offset; - newLength = elementsInfo.CalculateLength(newByteLength) - otherwise IfInvalidOffset; + newLength = ToSmiIndex( + Divide((Subtract(bufferByteLength, offset)), elementsInfo.size)) + otherwise IfInvalidLength; // 12. Else, } else { // a. Let newByteLength be newLength × elementSize. - newByteLength = elementsInfo.CalculateByteLength(newLength); + const newByteLength: Number = SmiMul(newLength, elementsInfo.size); // b. If offset + newByteLength > bufferByteLength, throw a RangeError // exception. - if ((bufferByteLength < newByteLength) || - (offset > bufferByteLength - newByteLength)) - goto IfInvalidLength; + const difference: Number = + Cast(Subtract(bufferByteLength, newByteLength)) + otherwise unreachable; + if (difference < offset) goto IfInvalidLength; } - SetupTypedArray(typedArray, newLength, offset, newByteLength); - AttachBuffer(typedArray, buffer, elementsInfo.map, newLength, offset); + TypedArrayInitializeWithBuffer( + typedArray, newLength, buffer, elementsInfo, offset); } label IfInvalidAlignment(problemString: String) deferred { ThrowInvalidTypedArrayAlignment(typedArray.map, problemString); diff --git a/src/builtins/typed-array.tq b/src/builtins/typed-array.tq index 866a127345..b9b0f605f1 100644 --- a/src/builtins/typed-array.tq +++ b/src/builtins/typed-array.tq @@ -6,38 +6,11 @@ namespace typed_array { struct TypedArrayElementsInfo { - // Calculates the number of bytes required for specified number of elements. - CalculateByteLength(length: uintptr): uintptr { - const byteLength = length << this.sizeLog2; - // Verify no overflow has ocurred. - assert(byteLength >>> this.sizeLog2 == length); - return byteLength; - } - CalculateByteLength(length: PositiveSmi): uintptr { - return this.CalculateByteLength(Convert(length)); - } - - // Calculates the maximum number of elements supported by a specified number - // of bytes. - CalculateLength(byteLength: uintptr): PositiveSmi labels IfInvalid { - return TryUintPtrToPositiveSmi(byteLength >>> this.sizeLog2) - otherwise IfInvalid; - } - - // Determines if `bytes` (byte offset or length) cannot be evenly divided by - // element size. - IsUnaligned(bytes: uintptr): bool { - // Exploits the fact the element size is a power of 2. Determining whether - // there is remainder (not aligned) can be achieved efficiently with bit - // masking. Shift is safe as sizeLog2 can be 3 at most (see - // ElementsKindToShiftSize). - return (bytes & ((1 << this.sizeLog2) - 1)) != 0; - } - - sizeLog2: uintptr; + size: PositiveSmi; map: Map; kind: ElementsKind; } + extern runtime TypedArraySortFast(Context, Object): JSTypedArray; extern macro TypedArrayBuiltinsAssembler::ValidateTypedArray( Context, Object, constexpr string): JSTypedArray; diff --git a/src/code-stub-assembler.cc b/src/code-stub-assembler.cc index 7b8f051969..eae96c6903 100644 --- a/src/code-stub-assembler.cc +++ b/src/code-stub-assembler.cc @@ -5691,28 +5691,20 @@ TNode CodeStubAssembler::ChangeNumberToFloat64( return result.value(); } -TNode CodeStubAssembler::TryNumberToUintPtr(TNode value, - Label* if_negative) { +TNode CodeStubAssembler::ChangeNonnegativeNumberToUintPtr( + TNode value) { TVARIABLE(UintPtrT, result); Label done(this, &result); Branch(TaggedIsSmi(value), [&] { TNode value_smi = CAST(value); - if (if_negative == nullptr) { - CSA_SLOW_ASSERT(this, SmiLessThan(SmiConstant(-1), value_smi)); - } else { - GotoIfNot(TaggedIsPositiveSmi(value), if_negative); - } + CSA_SLOW_ASSERT(this, SmiLessThan(SmiConstant(-1), value_smi)); result = UncheckedCast(SmiToIntPtr(value_smi)); Goto(&done); }, [&] { TNode value_hn = CAST(value); - TNode value = LoadHeapNumberValue(value_hn); - if (if_negative != nullptr) { - GotoIf(Float64LessThan(value, Float64Constant(0.0)), if_negative); - } - result = ChangeFloat64ToUintPtr(value); + result = ChangeFloat64ToUintPtr(LoadHeapNumberValue(value_hn)); Goto(&done); }); diff --git a/src/code-stub-assembler.h b/src/code-stub-assembler.h index 6b169b4694..53e1f5ea3a 100644 --- a/src/code-stub-assembler.h +++ b/src/code-stub-assembler.h @@ -1988,10 +1988,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler TNode ChangeUintPtrToTagged(TNode value); TNode ChangeNumberToUint32(TNode value); TNode ChangeNumberToFloat64(SloppyTNode value); - TNode TryNumberToUintPtr(TNode value, Label* if_negative); - TNode ChangeNonnegativeNumberToUintPtr(TNode value) { - return TryNumberToUintPtr(value, nullptr); - } + TNode ChangeNonnegativeNumberToUintPtr(TNode value); void TaggedToNumeric(Node* context, Node* value, Label* done, Variable* var_numeric); diff --git a/src/external-reference.cc b/src/external-reference.cc index 44efe5a6ee..5aa020084b 100644 --- a/src/external-reference.cc +++ b/src/external-reference.cc @@ -420,11 +420,6 @@ ExternalReference ExternalReference::address_of_min_int() { return ExternalReference(reinterpret_cast
(&double_min_int_constant)); } -ExternalReference -ExternalReference::address_of_mock_arraybuffer_allocator_flag() { - return ExternalReference(&FLAG_mock_arraybuffer_allocator); -} - ExternalReference ExternalReference::address_of_runtime_stats_flag() { return ExternalReference(&FLAG_runtime_stats); } diff --git a/src/external-reference.h b/src/external-reference.h index f22f989195..2151a76fda 100644 --- a/src/external-reference.h +++ b/src/external-reference.h @@ -92,8 +92,6 @@ class StatsCounter; V(address_of_harmony_await_optimization_flag, \ "FLAG_harmony_await_optimization") \ V(address_of_min_int, "LDoubleConstant::min_int") \ - V(address_of_mock_arraybuffer_allocator_flag, \ - "FLAG_mock_arraybuffer_allocator") \ V(address_of_one_half, "LDoubleConstant::one_half") \ V(address_of_runtime_stats_flag, "FLAG_runtime_stats") \ V(address_of_the_hole_nan, "the_hole_nan") \