Skip to content

Commit

Permalink
[csa][cleanup] Remove ParameterMode from the codebase
Browse files Browse the repository at this point in the history 
Remove ParameterMode/Tnodify StoreFixedArrayOrPropertyArrayElement
which had the last uses of:
 * ElementOffsetFromIndex
 * MatchesParameterMode

So we can clean those methods too.

With all of this combined, we can remove the ParameterMode declaration
from the codebase.

Bug: v8:9708, v8:6949
Change-Id: I981608681cefafe910dd40d3b82f8252e4b8994d
Fixes: v8:9708
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2379514
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69586}
  • Loading branch information
@santiaboy
santiaboy authored and Commit Bot committed Aug 27, 2020
1 parent 6ad8193 commit f2851de
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 62 deletions.
56 changes: 25 additions & 31 deletions src/codegen/code-stub-assembler.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,14 +318,6 @@ TNode<IntPtrT> CodeStubAssembler::IntPtrRoundUpToPowerOfTwo32(
return Signed(IntPtrAdd(value, IntPtrConstant(1)));
}

Node* CodeStubAssembler::MatchesParameterMode(Node* value, ParameterMode mode) {
if (mode == SMI_PARAMETERS) {
return TaggedIsSmi(value);
} else {
return Int32Constant(1);
}
}

TNode<BoolT> CodeStubAssembler::WordIsPowerOfTwo(SloppyTNode<IntPtrT> value) {
intptr_t constant;
if (ToIntPtrConstant(value, &constant)) {
Expand Down Expand Up @@ -2700,13 +2692,15 @@ void CodeStubAssembler::StoreObjectFieldRoot(TNode<HeapObject> object,
}
}

template <typename TIndex>
void CodeStubAssembler::StoreFixedArrayOrPropertyArrayElement(
TNode<UnionT<FixedArray, PropertyArray>> object, Node* index_node,
TNode<Object> value, WriteBarrierMode barrier_mode, int additional_offset,
ParameterMode parameter_mode) {
CSA_SLOW_ASSERT(
this, Word32Or(IsFixedArraySubclass(object), IsPropertyArray(object)));
CSA_SLOW_ASSERT(this, MatchesParameterMode(index_node, parameter_mode));
TNode<UnionT<FixedArray, PropertyArray>> object, TNode<TIndex> index_node,
TNode<Object> value, WriteBarrierMode barrier_mode, int additional_offset) {
// TODO(v8:9708): Do we want to keep both IntPtrT and UintPtrT variants?
static_assert(std::is_same<TIndex, Smi>::value ||
std::is_same<TIndex, UintPtrT>::value ||
std::is_same<TIndex, IntPtrT>::value,
"Only Smi, UintPtrT or IntPtrT index is allowed");
DCHECK(barrier_mode == SKIP_WRITE_BARRIER ||
barrier_mode == UNSAFE_SKIP_WRITE_BARRIER ||
barrier_mode == UPDATE_WRITE_BARRIER ||
Expand All @@ -2716,8 +2710,8 @@ void CodeStubAssembler::StoreFixedArrayOrPropertyArrayElement(
static_cast<int>(PropertyArray::kHeaderSize));
int header_size =
FixedArray::kHeaderSize + additional_offset - kHeapObjectTag;
TNode<IntPtrT> offset = ElementOffsetFromIndex(index_node, HOLEY_ELEMENTS,
parameter_mode, header_size);
TNode<IntPtrT> offset =
ElementOffsetFromIndex(index_node, HOLEY_ELEMENTS, header_size);
STATIC_ASSERT(static_cast<int>(FixedArrayBase::kLengthOffset) ==
static_cast<int>(WeakFixedArray::kLengthOffset));
STATIC_ASSERT(static_cast<int>(FixedArrayBase::kLengthOffset) ==
Expand Down Expand Up @@ -2753,6 +2747,21 @@ void CodeStubAssembler::StoreFixedArrayOrPropertyArrayElement(
}
}

template V8_EXPORT_PRIVATE void
CodeStubAssembler::StoreFixedArrayOrPropertyArrayElement<Smi>(
TNode<UnionT<FixedArray, PropertyArray>>, TNode<Smi>, TNode<Object>,
WriteBarrierMode, int);

template V8_EXPORT_PRIVATE void
CodeStubAssembler::StoreFixedArrayOrPropertyArrayElement<IntPtrT>(
TNode<UnionT<FixedArray, PropertyArray>>, TNode<IntPtrT>, TNode<Object>,
WriteBarrierMode, int);

template V8_EXPORT_PRIVATE void
CodeStubAssembler::StoreFixedArrayOrPropertyArrayElement<UintPtrT>(
TNode<UnionT<FixedArray, PropertyArray>>, TNode<UintPtrT>, TNode<Object>,
WriteBarrierMode, int);

template <typename TIndex>
void CodeStubAssembler::StoreFixedDoubleArrayElement(
TNode<FixedDoubleArray> object, TNode<TIndex> index, TNode<Float64T> value,
Expand Down Expand Up @@ -9162,21 +9171,6 @@ TNode<Oddball> CodeStubAssembler::OrdinaryHasInstance(
return var_result.value();
}

TNode<IntPtrT> CodeStubAssembler::ElementOffsetFromIndex(Node* index_node,
ElementsKind kind,
ParameterMode mode,
int base_size) {
CSA_SLOW_ASSERT(this, MatchesParameterMode(index_node, mode));
if (mode == SMI_PARAMETERS) {
return ElementOffsetFromIndex(ReinterpretCast<Smi>(index_node), kind,
base_size);
} else {
DCHECK(mode == INTPTR_PARAMETERS);
return ElementOffsetFromIndex(ReinterpretCast<IntPtrT>(index_node), kind,
base_size);
}
}

template <typename TIndex>
TNode<IntPtrT> CodeStubAssembler::ElementOffsetFromIndex(
TNode<TIndex> index_node, ElementsKind kind, int base_size) {
Expand Down
37 changes: 6 additions & 31 deletions src/codegen/code-stub-assembler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,24 +318,6 @@ class V8_EXPORT_PRIVATE CodeStubAssembler

using AllocationFlags = base::Flags<AllocationFlag>;

enum ParameterMode { SMI_PARAMETERS, INTPTR_PARAMETERS };

// On 32-bit platforms, there is a slight performance advantage to doing all
// of the array offset/index arithmetic with SMIs, since it's possible
// to save a few tag/untag operations without paying an extra expense when
// calculating array offset (the smi math can be folded away) and there are
// fewer live ranges. Thus only convert indices to untagged value on 64-bit
// platforms.
ParameterMode OptimalParameterMode() const {
#if defined(BINT_IS_SMI)
return SMI_PARAMETERS;
#elif defined(BINT_IS_INTPTR)
return INTPTR_PARAMETERS;
#else
#error Unknown BInt type.
#endif
}

TNode<IntPtrT> ParameterToIntPtr(TNode<Smi> value) { return SmiUntag(value); }
TNode<IntPtrT> ParameterToIntPtr(TNode<IntPtrT> value) { return value; }
TNode<IntPtrT> ParameterToIntPtr(TNode<UintPtrT> value) {
Expand Down Expand Up @@ -458,8 +440,6 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
return CAST(heap_object);
}

Node* MatchesParameterMode(Node* value, ParameterMode mode);

#define PARAMETER_BINOP(OpName, IntPtrOpName, SmiOpName) \
TNode<Smi> OpName(TNode<Smi> a, TNode<Smi> b) { return SmiOpName(a, b); } \
TNode<IntPtrT> OpName(TNode<IntPtrT> a, TNode<IntPtrT> b) { \
Expand Down Expand Up @@ -1523,13 +1503,11 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
std::is_same<TIndex, UintPtrT>::value ||
std::is_same<TIndex, IntPtrT>::value,
"Only Smi, UintPtrT or IntPtrT index is allowed");
const ParameterMode mode =
std::is_same<TIndex, Smi>::value ? SMI_PARAMETERS : INTPTR_PARAMETERS;
if (NeedsBoundsCheck(check_bounds)) {
FixedArrayBoundsCheck(array, index, additional_offset);
}
StoreFixedArrayOrPropertyArrayElement(array, index, value, barrier_mode,
additional_offset, 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.
Expand All @@ -1551,8 +1529,8 @@ class V8_EXPORT_PRIVATE CodeStubAssembler

void StorePropertyArrayElement(TNode<PropertyArray> array,
TNode<IntPtrT> index, TNode<Object> value) {
StoreFixedArrayOrPropertyArrayElement(
array, index, value, UPDATE_WRITE_BARRIER, 0, INTPTR_PARAMETERS);
StoreFixedArrayOrPropertyArrayElement(array, index, value,
UPDATE_WRITE_BARRIER);
}

void StoreFixedArrayElement(
Expand Down Expand Up @@ -3347,9 +3325,6 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
template <typename TIndex>
TNode<IntPtrT> ElementOffsetFromIndex(TNode<TIndex> index, ElementsKind kind,
int base_size = 0);
// TODO(v8:9708): remove once all uses are ported.
TNode<IntPtrT> ElementOffsetFromIndex(Node* index, ElementsKind kind,
ParameterMode mode, int base_size = 0);

// Check that a field offset is within the bounds of the an object.
TNode<BoolT> IsOffsetInBounds(SloppyTNode<IntPtrT> offset,
Expand Down Expand Up @@ -3659,11 +3634,11 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
return CodeAssembler::LoadRoot(root_index);
}

template <typename TIndex>
void StoreFixedArrayOrPropertyArrayElement(
TNode<UnionT<FixedArray, PropertyArray>> array, Node* index,
TNode<UnionT<FixedArray, PropertyArray>> array, TNode<TIndex> index,
TNode<Object> value, WriteBarrierMode barrier_mode = UPDATE_WRITE_BARRIER,
int additional_offset = 0,
ParameterMode parameter_mode = INTPTR_PARAMETERS);
int additional_offset = 0);
};

class V8_EXPORT_PRIVATE CodeStubArguments {
Expand Down

0 comments on commit f2851de

Please sign in to comment.