From a786179c4742593559d39b3158e9d091aec094ad Mon Sep 17 00:00:00 2001 From: Santiago Aboy Solanes Date: Fri, 12 Feb 2021 15:15:52 +0000 Subject: [PATCH] [csa][cleanup] Simplify StoreFixedArrayElement We can remove some of the method definitions, as well as the sloppy-ness from the method. Bug: v8:6949, v8:11384 Change-Id: I04880daa3fcce097b79009f12bd24128a47c2c80 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2690591 Reviewed-by: Michael Stanton Reviewed-by: Ross McIlroy Reviewed-by: Mythri Alle Commit-Queue: Santiago Aboy Solanes Cr-Commit-Position: refs/heads/master@{#72867} --- src/builtins/array-reverse.tq | 2 +- src/codegen/code-stub-assembler.h | 48 +++++++++------------ test/cctest/compiler/test-code-generator.cc | 2 +- test/cctest/test-code-stub-assembler.cc | 3 +- third_party/v8/builtins/array-sort.tq | 2 +- 5 files changed, 26 insertions(+), 31 deletions(-) diff --git a/src/builtins/array-reverse.tq b/src/builtins/array-reverse.tq index fe9df2b9b5..b154483d06 100644 --- a/src/builtins/array-reverse.tq +++ b/src/builtins/array-reverse.tq @@ -32,7 +32,7 @@ macro StoreElement( StoreElement(implicit context: Context)( elements: FixedArrayBase, index: Smi, value: Smi) { const elems: FixedArray = UnsafeCast(elements); - StoreFixedArrayElement(elems, index, value, SKIP_WRITE_BARRIER); + StoreFixedArrayElement(elems, index, value); } StoreElement(implicit context: Context)( diff --git a/src/codegen/code-stub-assembler.h b/src/codegen/code-stub-assembler.h index 9bed554b7d..c50b7f9a92 100644 --- a/src/codegen/code-stub-assembler.h +++ b/src/codegen/code-stub-assembler.h @@ -1555,37 +1555,43 @@ class V8_EXPORT_PRIVATE CodeStubAssembler void StoreMapNoWriteBarrier(TNode object, TNode map); void StoreObjectFieldRoot(TNode object, int offset, RootIndex root); + // Store an array element to a FixedArray. void StoreFixedArrayElement( - TNode object, int index, SloppyTNode value, + TNode object, int index, TNode value, WriteBarrierMode barrier_mode = UPDATE_WRITE_BARRIER, CheckBounds check_bounds = CheckBounds::kAlways) { return StoreFixedArrayElement(object, IntPtrConstant(index), value, barrier_mode, 0, check_bounds); } + // This doesn't emit a bounds-check. As part of the security-performance // tradeoff, only use it if it is performance critical. void UnsafeStoreFixedArrayElement( TNode object, int index, TNode value, WriteBarrierMode barrier_mode = UPDATE_WRITE_BARRIER) { - return StoreFixedArrayElement(object, index, value, barrier_mode, - CheckBounds::kDebugOnly); + return StoreFixedArrayElement(object, IntPtrConstant(index), value, + barrier_mode, 0, CheckBounds::kDebugOnly); } + void UnsafeStoreFixedArrayElement(TNode object, int index, TNode value) { - return StoreFixedArrayElement(object, index, value, - UNSAFE_SKIP_WRITE_BARRIER, + return StoreFixedArrayElement(object, IntPtrConstant(index), value, + UNSAFE_SKIP_WRITE_BARRIER, 0, CheckBounds::kDebugOnly); } + void StoreFixedArrayElement(TNode object, int index, TNode value, CheckBounds check_bounds = CheckBounds::kAlways) { - return StoreFixedArrayElement(object, IntPtrConstant(index), value, + return StoreFixedArrayElement(object, IntPtrConstant(index), + TNode{value}, UNSAFE_SKIP_WRITE_BARRIER, 0, check_bounds); } + template void StoreFixedArrayElement( - TNode array, TNode index, SloppyTNode value, + TNode array, TNode index, TNode value, WriteBarrierMode barrier_mode = UPDATE_WRITE_BARRIER, int additional_offset = 0, CheckBounds check_bounds = CheckBounds::kAlways) { @@ -1600,6 +1606,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler StoreFixedArrayOrPropertyArrayElement(array, index, value, barrier_mode, additional_offset); } + // This doesn't emit a bounds-check. As part of the security-performance // tradeoff, only use it if it is performance critical. void UnsafeStoreFixedArrayElement( @@ -1624,24 +1631,12 @@ class V8_EXPORT_PRIVATE CodeStubAssembler UPDATE_WRITE_BARRIER); } - void StoreFixedArrayElement( - TNode array, TNode index, TNode value, - WriteBarrierMode barrier_mode = UPDATE_WRITE_BARRIER) { - StoreFixedArrayElement(array, index, value, barrier_mode, 0); - } - void StoreFixedArrayElement( - TNode array, TNode index, TNode value, - WriteBarrierMode barrier_mode = SKIP_WRITE_BARRIER, - int additional_offset = 0) { - DCHECK_EQ(SKIP_WRITE_BARRIER, barrier_mode); - StoreFixedArrayElement(array, index, TNode{value}, - UNSAFE_SKIP_WRITE_BARRIER, additional_offset); - } - void StoreFixedArrayElement( - TNode array, TNode index, TNode value, - WriteBarrierMode barrier_mode = SKIP_WRITE_BARRIER, - int additional_offset = 0) { - DCHECK_EQ(SKIP_WRITE_BARRIER, barrier_mode); + template + void StoreFixedArrayElement(TNode array, TNode index, + TNode value, int additional_offset = 0) { + static_assert(std::is_same::value || + std::is_same::value, + "Only Smi or IntPtrT indeces is allowed"); StoreFixedArrayElement(array, index, TNode{value}, UNSAFE_SKIP_WRITE_BARRIER, additional_offset); } @@ -2862,8 +2857,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler const int kKeyToDetailsOffset = (ContainerType::kEntryDetailsIndex - ContainerType::kEntryKeyIndex) * kTaggedSize; - StoreFixedArrayElement(container, key_index, details, SKIP_WRITE_BARRIER, - kKeyToDetailsOffset); + StoreFixedArrayElement(container, key_index, details, kKeyToDetailsOffset); } // Stores the value for the entry with the given key_index. diff --git a/test/cctest/compiler/test-code-generator.cc b/test/cctest/compiler/test-code-generator.cc index f91dbac4b8..27ad187552 100644 --- a/test/cctest/compiler/test-code-generator.cc +++ b/test/cctest/compiler/test-code-generator.cc @@ -211,7 +211,7 @@ Handle BuildTeardownFunction(Isolate* isolate, Node* param = __ UntypedParameter(i + 2); switch (parameters[i].representation()) { case MachineRepresentation::kTagged: - __ StoreFixedArrayElement(result_array, i, param, + __ StoreFixedArrayElement(result_array, i, __ Cast(param), UNSAFE_SKIP_WRITE_BARRIER); break; // Box FP values into HeapNumbers. diff --git a/test/cctest/test-code-stub-assembler.cc b/test/cctest/test-code-stub-assembler.cc index 54b6a9e5b2..758986038a 100644 --- a/test/cctest/test-code-stub-assembler.cc +++ b/test/cctest/test-code-stub-assembler.cc @@ -2748,7 +2748,8 @@ TEST(CreatePromiseResolvingFunctions) { m.NewJSPromise(context, m.UndefinedConstant()); PromiseResolvingFunctions funcs = m.CreatePromiseResolvingFunctions( context, promise, m.BooleanConstant(false), native_context); - Node *resolve = funcs.resolve, *reject = funcs.reject; + TNode resolve = funcs.resolve; + TNode reject = funcs.reject; TNode const kSize = m.IntPtrConstant(2); TNode const arr = m.Cast(m.AllocateFixedArray(PACKED_ELEMENTS, kSize)); diff --git a/third_party/v8/builtins/array-sort.tq b/third_party/v8/builtins/array-sort.tq index ea7c0e7dc9..7737ab78e3 100644 --- a/third_party/v8/builtins/array-sort.tq +++ b/third_party/v8/builtins/array-sort.tq @@ -276,7 +276,7 @@ Store( const object = UnsafeCast(sortState.receiver); const elements = UnsafeCast(object.elements); const value = UnsafeCast(value); - StoreFixedArrayElement(elements, index, value, SKIP_WRITE_BARRIER); + StoreFixedArrayElement(elements, index, value); return kSuccess; }