Commit d5d0484d authored by Benedikt Meurer's avatar Benedikt Meurer Committed by Commit Bot

[es2015] Restore %ArrayIteratorPrototype%.next() baseline performance.

With the previous changes the builtin would take the slow path for
JSArray's when the iterator was already exhausted (i.e. the internal
[[ArrayIteratorNextIndex]] field contains 2^32-1 as HeapNumber), even
though that could also be handled in the fast path. This changes the
handling such that the three distinct cases (if_array, if_other and
if_typedarray) are really distinct, and all JSArray's are always
handled by the if_array case.

Bug: v8:7510, v8:7514, v8:8070, chromium:876654
Change-Id: I1636b0616645f9e99f34f851df410992653cb380
Reviewed-on: https://chromium-review.googlesource.com/1186403Reviewed-by: 's avatarSigurd Schneider <sigurds@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55341}
parent a984ccd7
......@@ -3534,7 +3534,6 @@ TF_BUILTIN(ArrayIteratorPrototypeNext, CodeStubAssembler) {
TNode<Number> index =
CAST(LoadObjectField(iterator, JSArrayIterator::kNextIndexOffset));
CSA_ASSERT(this, IsNumberNonNegativeSafeInteger(index));
GotoIfNot(TaggedIsSmi(index), &if_other);
// Dispatch based on the type of the {array}.
TNode<Map> array_map = LoadMap(array);
......@@ -3545,11 +3544,18 @@ TF_BUILTIN(ArrayIteratorPrototypeNext, CodeStubAssembler) {
BIND(&if_array);
{
// Check that the {index} is within range for the {array}.
TNode<Number> length = LoadJSArrayLength(CAST(array));
GotoIfNumberGreaterThanOrEqual(index, length, &set_done);
StoreObjectFieldNoWriteBarrier(iterator, JSArrayIterator::kNextIndexOffset,
SmiInc(CAST(index)));
// If {array} is a JSArray, then the {index} must be in Unsigned32 range.
CSA_ASSERT(this, IsNumberArrayIndex(index));
// Check that the {index} is within range for the {array}. We handle all
// kinds of JSArray's here, so we do the computation on Uint32.
TNode<Uint32T> index32 = ChangeNumberToUint32(index);
TNode<Uint32T> length32 =
ChangeNumberToUint32(LoadJSArrayLength(CAST(array)));
GotoIfNot(Uint32LessThan(index32, length32), &set_done);
StoreObjectField(
iterator, JSArrayIterator::kNextIndexOffset,
ChangeUint32ToTagged(Unsigned(Int32Add(index32, Int32Constant(1)))));
var_done.Bind(FalseConstant());
var_value.Bind(index);
......@@ -3563,7 +3569,8 @@ TF_BUILTIN(ArrayIteratorPrototypeNext, CodeStubAssembler) {
TNode<Int32T> elements_kind = LoadMapElementsKind(array_map);
TNode<FixedArrayBase> elements = LoadElements(CAST(array));
var_value.Bind(LoadFixedArrayBaseElementAsTagged(
elements, CAST(index), elements_kind, &if_generic, &if_hole));
elements, Signed(ChangeUint32ToWord(index32)), elements_kind,
&if_generic, &if_hole));
Goto(&allocate_entry_if_needed);
BIND(&if_hole);
......@@ -3576,17 +3583,9 @@ TF_BUILTIN(ArrayIteratorPrototypeNext, CodeStubAssembler) {
BIND(&if_other);
{
// If the {array} is actually a JSArray the {index} must be a valid
// array index, as the TurboFan fast-path inlining relies on the fact
// that the [[ArrayIteratorNextIndex]] field always contains a valid
// Unsigned32 value as long as the [[ArrayIteratorIteratedObject]]
// field contains a JSArray instance. Also rule out JSTypedArray's
// here as they should never reach here (both because the {index}
// in that case must always be a Smi, and second because loading
// the "length" property would be wrong for JSTypedArray's).
// We cannot enter here with either JSArray's or JSTypedArray's.
CSA_ASSERT(this, Word32BinaryNot(IsJSArray(array)));
CSA_ASSERT(this, Word32BinaryNot(IsJSTypedArray(array)));
CSA_ASSERT(this, Word32Or(Word32BinaryNot(IsJSArray(array)),
IsNumberArrayIndex(index)));
// Check that the {index} is within the bounds of the {array}s "length".
TNode<Number> length = CAST(
......@@ -3639,6 +3638,9 @@ TF_BUILTIN(ArrayIteratorPrototypeNext, CodeStubAssembler) {
BIND(&if_typedarray);
{
// If {array} is a JSTypedArray, the {index} must always be a Smi.
CSA_ASSERT(this, TaggedIsSmi(index));
// Check that the {array}s buffer wasn't neutered.
ThrowIfArrayBufferViewBufferIsDetached(context, CAST(array), method_name);
......
......@@ -2383,7 +2383,7 @@ TNode<Float64T> CodeStubAssembler::LoadFixedDoubleArrayElement(
}
TNode<Object> CodeStubAssembler::LoadFixedArrayBaseElementAsTagged(
TNode<FixedArrayBase> elements, TNode<Smi> index,
TNode<FixedArrayBase> elements, TNode<IntPtrT> index,
TNode<Int32T> elements_kind, Label* if_accessor, Label* if_hole) {
TVARIABLE(Object, var_result);
Label done(this), if_packed(this), if_holey(this), if_packed_double(this),
......@@ -2409,29 +2409,27 @@ TNode<Object> CodeStubAssembler::LoadFixedArrayBaseElementAsTagged(
BIND(&if_packed);
{
var_result =
LoadFixedArrayElement(CAST(elements), index, 0, SMI_PARAMETERS);
var_result = LoadFixedArrayElement(CAST(elements), index, 0);
Goto(&done);
}
BIND(&if_holey);
{
var_result =
LoadFixedArrayElement(CAST(elements), index, 0, SMI_PARAMETERS);
var_result = LoadFixedArrayElement(CAST(elements), index);
Branch(WordEqual(var_result.value(), TheHoleConstant()), if_hole, &done);
}
BIND(&if_packed_double);
{
var_result = AllocateHeapNumberWithValue(LoadFixedDoubleArrayElement(
CAST(elements), index, MachineType::Float64(), 0, SMI_PARAMETERS));
CAST(elements), index, MachineType::Float64()));
Goto(&done);
}
BIND(&if_holey_double);
{
var_result = AllocateHeapNumberWithValue(LoadFixedDoubleArrayElement(
CAST(elements), index, MachineType::Float64(), 0, SMI_PARAMETERS,
CAST(elements), index, MachineType::Float64(), 0, INTPTR_PARAMETERS,
if_hole));
Goto(&done);
}
......@@ -2439,8 +2437,8 @@ TNode<Object> CodeStubAssembler::LoadFixedArrayBaseElementAsTagged(
BIND(&if_dictionary);
{
CSA_ASSERT(this, IsDictionaryElementsKind(elements_kind));
var_result = BasicLoadNumberDictionaryElement(
CAST(elements), SmiToIntPtr(index), if_accessor, if_hole);
var_result = BasicLoadNumberDictionaryElement(CAST(elements), index,
if_accessor, if_hole);
Goto(&done);
}
......@@ -5002,6 +5000,24 @@ TNode<String> CodeStubAssembler::ToThisString(Node* context, Node* value,
return CAST(var_value.value());
}
TNode<Uint32T> CodeStubAssembler::ChangeNumberToUint32(TNode<Number> value) {
TVARIABLE(Uint32T, var_result);
Label if_smi(this), if_heapnumber(this, Label::kDeferred), done(this);
Branch(TaggedIsSmi(value), &if_smi, &if_heapnumber);
BIND(&if_smi);
{
var_result = Unsigned(SmiToInt32(CAST(value)));
Goto(&done);
}
BIND(&if_heapnumber);
{
var_result = ChangeFloat64ToUint32(LoadHeapNumberValue(CAST(value)));
Goto(&done);
}
BIND(&done);
return var_result.value();
}
TNode<Float64T> CodeStubAssembler::ChangeNumberToFloat64(
SloppyTNode<Number> value) {
// TODO(tebbi): Remove assert once argument is TNode instead of SloppyTNode.
......
......@@ -1063,7 +1063,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
// was found is an accessor, or to |if_hole| if the element at
// the given |index| is not found in |elements|.
TNode<Object> LoadFixedArrayBaseElementAsTagged(
TNode<FixedArrayBase> elements, TNode<Smi> index,
TNode<FixedArrayBase> elements, TNode<IntPtrT> index,
TNode<Int32T> elements_kind, Label* if_accessor, Label* if_hole);
// Load a feedback slot from a FeedbackVector.
......@@ -1661,6 +1661,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
TNode<Number> ChangeFloat64ToTagged(SloppyTNode<Float64T> value);
TNode<Number> ChangeInt32ToTagged(SloppyTNode<Int32T> value);
TNode<Number> ChangeUint32ToTagged(SloppyTNode<Uint32T> value);
TNode<Uint32T> ChangeNumberToUint32(TNode<Number> value);
TNode<Float64T> ChangeNumberToFloat64(SloppyTNode<Number> value);
TNode<UintPtrT> ChangeNonnegativeNumberToUintPtr(TNode<Number> value);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment