Skip to content

V
V8
Commit 933e3d8b authored by Leszek Swirski's avatar Leszek Swirski Committed by V8 LUCI CQ
Browse files
Options

Revert "[maglev] Optimize monomorphic keyed loads" 

This reverts commit 133e7f83.

Reason for revert: Breaks compilation for non-pointer-compressed x64

Original change's description:
> [maglev] Optimize monomorphic keyed loads
>
> Add a fast path for keyed loads that are:
>
>   1. Monomorphic,
>   2. Fast elements accesses,
>   3. Not out-of-bounds (deopt on OOB),
>   4. Not holey
>
> Bug: v8:7700
> Change-Id: I4d46f4d0ce7065c93a9b092833fb16a8c9e9f94e
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3882974
> Auto-Submit: Leszek Swirski <leszeks@chromium.org>
> Reviewed-by: Jakob Linke <jgruber@chromium.org>
> Commit-Queue: Leszek Swirski <leszeks@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#83149}

Bug: v8:7700
Change-Id: I08e7ca3a79b383d19c6baf73a721364b859d6df3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3890916
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#83150}
parent 133e7f83
Hide whitespace changes
Inline Side-by-side
Showing with 1 addition and 287 deletions
src/diagnostics/objects-printer.cc
View file @ 933e3d8b
......@@ -1265,6 +1265,7 @@ void FeedbackNexus::Print(std::ostream& os) {
case FeedbackSlotKind::kDefineKeyedOwn:
case FeedbackSlotKind::kHasKeyed:
case FeedbackSlotKind::kInstanceOf:
case FeedbackSlotKind::kLoadKeyed:
case FeedbackSlotKind::kDefineKeyedOwnPropertyInLiteral:
case FeedbackSlotKind::kStoreGlobalSloppy:
case FeedbackSlotKind::kStoreGlobalStrict:
......@@ -1290,7 +1291,6 @@ void FeedbackNexus::Print(std::ostream& os) {
}
break;
}
case FeedbackSlotKind::kLoadKeyed:
case FeedbackSlotKind::kLoadProperty: {
os << InlineCacheState2String(ic_state());
if (ic_state() == InlineCacheState::MONOMORPHIC) {
......
src/maglev/maglev-graph-builder.cc
View file @ 933e3d8b
......@@ -22,7 +22,6 @@
#include "src/maglev/maglev-compilation-unit.h"
#include "src/maglev/maglev-interpreter-frame-state.h"
#include "src/maglev/maglev-ir.h"
#include "src/objects/elements-kind.h"
#include "src/objects/feedback-vector.h"
#include "src/objects/literal-objects-inl.h"
#include "src/objects/name-inl.h"
......@@ -1134,60 +1133,6 @@ bool MaglevGraphBuilder::TryBuildMonomorphicLoadFromLoadHandler(
return true;
}
bool MaglevGraphBuilder::TryBuildMonomorphicElementLoad(
ValueNode* object, ValueNode* index, const compiler::MapRef& map,
MaybeObjectHandle handler) {
if (handler.is_null()) return false;
if (handler->IsSmi()) {
return TryBuildMonomorphicElementLoadFromSmiHandler(
object, index, map, handler->ToSmi().value());
}
return false;
}
bool MaglevGraphBuilder::TryBuildMonomorphicElementLoadFromSmiHandler(
ValueNode* object, ValueNode* index, const compiler::MapRef& map,
int32_t handler) {
LoadHandler::Kind kind = LoadHandler::KindBits::decode(handler);
switch (kind) {
case LoadHandler::Kind::kElement: {
if (LoadHandler::AllowOutOfBoundsBits::decode(handler)) {
return false;
}
ElementsKind elements_kind =
LoadHandler::ElementsKindBits::decode(handler);
if (!IsFastElementsKind(elements_kind)) return false;
// TODO(leszeks): Handle holey elements.
if (IsHoleyElementsKind(elements_kind)) return false;
DCHECK(!LoadHandler::ConvertHoleBits::decode(handler));
BuildMapCheck(object, map);
BuildCheckSmi(index);
if (LoadHandler::IsJsArrayBits::decode(handler)) {
DCHECK(map.IsJSArrayMap());
AddNewNode<CheckJSArrayBounds>({object, index});
} else {
DCHECK(!map.IsJSArrayMap());
DCHECK(map.IsJSObjectMap());
AddNewNode<CheckJSObjectElementsBounds>({object, index});
}
if (elements_kind == ElementsKind::PACKED_DOUBLE_ELEMENTS) {
SetAccumulator(AddNewNode<LoadDoubleElement>({object, index}));
} else {
DCHECK(!IsDoubleElementsKind(elements_kind));
SetAccumulator(AddNewNode<LoadTaggedElement>({object, index}));
}
return true;
}
default:
return false;
}
}
void MaglevGraphBuilder::VisitGetNamedProperty() {
// GetNamedProperty <object> <name_index> <slot>
ValueNode* object = LoadRegisterTagged(0);
......@@ -1281,8 +1226,6 @@ void MaglevGraphBuilder::VisitGetNamedPropertyFromSuper() {
void MaglevGraphBuilder::VisitGetKeyedProperty() {
// GetKeyedProperty <object> <slot>
ValueNode* object = LoadRegisterTagged(0);
// TODO(leszeks): We don't need to tag the key if it's an Int32 and a simple
// monomorphic element load.
ValueNode* key = GetAccumulatorTagged();
FeedbackSlot slot = GetSlotOperand(1);
compiler::FeedbackSource feedback_source{feedback(), slot};
......@@ -1297,22 +1240,6 @@ void MaglevGraphBuilder::VisitGetKeyedProperty() {
DeoptimizeReason::kInsufficientTypeFeedbackForGenericKeyedAccess);
return;
case compiler::ProcessedFeedback::kElementAccess: {
const compiler::ElementAccessFeedback& element_feedback =
processed_feedback.AsElementAccess();
if (element_feedback.transition_groups().size() != 1) break;
compiler::MapRef map = MakeRefAssumeMemoryFence(
broker(), element_feedback.transition_groups()[0].front());
// Monomorphic load, check the handler.
// TODO(leszeks): Make GetFeedbackForPropertyAccess read the handler.
MaybeObjectHandle handler =
FeedbackNexusForSlot(slot).FindHandlerForMap(map.object());
if (TryBuildMonomorphicElementLoad(object, key, map, handler)) return;
break;
}
default:
break;
}
......
src/maglev/maglev-graph-builder.h
View file @ 933e3d8b
......@@ -969,14 +969,6 @@ class MaglevGraphBuilder {
const compiler::MapRef& map,
LoadHandler handler);
bool TryBuildMonomorphicElementLoad(ValueNode* object, ValueNode* index,
const compiler::MapRef& map,
MaybeObjectHandle handler);
bool TryBuildMonomorphicElementLoadFromSmiHandler(ValueNode* object,
ValueNode* index,
const compiler::MapRef& map,
int32_t handler);
bool TryBuildMonomorphicStore(ValueNode* object, const compiler::MapRef& map,
MaybeObjectHandle handler);
bool TryBuildMonomorphicStoreFromSmiHandler(ValueNode* object,
......
src/maglev/maglev-graph-verifier.h
View file @ 933e3d8b
......@@ -175,10 +175,6 @@ class MaglevGraphVerifier {
case Opcode::kGenericLessThan:
case Opcode::kGenericLessThanOrEqual:
case Opcode::kGenericStrictEqual:
case Opcode::kCheckJSArrayBounds:
case Opcode::kCheckJSObjectElementsBounds:
case Opcode::kLoadTaggedElement:
case Opcode::kLoadDoubleElement:
case Opcode::kGetIterator:
case Opcode::kTaggedEqual:
case Opcode::kTaggedNotEqual:
......
src/maglev/maglev-ir.cc
View file @ 933e3d8b
......@@ -28,7 +28,6 @@
#include "src/maglev/maglev-interpreter-frame-state.h"
#include "src/maglev/maglev-ir-inl.h"
#include "src/maglev/maglev-vreg-allocator.h"
#include "src/objects/instance-type.h"
namespace v8 {
namespace internal {
......@@ -1352,56 +1351,6 @@ void CheckMapsWithMigration::PrintParams(
os << "(" << *map().object() << ")";
}
void CheckJSArrayBounds::AllocateVreg(MaglevVregAllocationState* vreg_state) {
UseRegister(receiver_input());
UseRegister(index_input());
}
void CheckJSArrayBounds::GenerateCode(MaglevAssembler* masm,
const ProcessingState& state) {
Register object = ToRegister(receiver_input());
Register index = ToRegister(index_input());
__ AssertNotSmi(object);
__ AssertSmi(index);
if (FLAG_debug_code) {
__ CmpObjectType(object, JS_ARRAY_TYPE, kScratchRegister);
__ Assert(equal, AbortReason::kUnexpectedValue);
}
TaggedRegister length(kScratchRegister);
__ LoadAnyTaggedField(length, FieldOperand(object, JSArray::kLengthOffset));
__ cmp_tagged(index, length.reg());
__ EmitEagerDeoptIf(above_equal, DeoptimizeReason::kOutOfBounds, this);
}
void CheckJSObjectElementsBounds::AllocateVreg(
MaglevVregAllocationState* vreg_state) {
UseRegister(receiver_input());
UseRegister(index_input());
}
void CheckJSObjectElementsBounds::GenerateCode(MaglevAssembler* masm,
const ProcessingState& state) {
Register object = ToRegister(receiver_input());
Register index = ToRegister(index_input());
__ AssertNotSmi(object);
__ AssertSmi(index);
if (FLAG_debug_code) {
__ CmpObjectType(object, FIRST_JS_OBJECT_TYPE, kScratchRegister);
__ Assert(greater_equal, AbortReason::kUnexpectedValue);
}
__ LoadAnyTaggedField(
kScratchRegister,
FieldOperand(object, JSReceiver::kPropertiesOrHashOffset));
if (FLAG_debug_code) {
__ AssertNotSmi(kScratchRegister);
}
TaggedRegister length(kScratchRegister);
__ LoadAnyTaggedField(
length, FieldOperand(kScratchRegister, FixedArray::kLengthOffset));
__ cmp_tagged(index, length.reg());
__ EmitEagerDeoptIf(above_equal, DeoptimizeReason::kOutOfBounds, this);
}
void CheckedInternalizedString::AllocateVreg(
MaglevVregAllocationState* vreg_state) {
UseRegister(object_input());
......@@ -1491,86 +1440,6 @@ void LoadDoubleField::PrintParams(std::ostream& os,
os << "(0x" << std::hex << offset() << std::dec << ")";
}
void LoadTaggedElement::AllocateVreg(MaglevVregAllocationState* vreg_state) {
UseRegister(object_input());
UseRegister(index_input());
DefineAsRegister(vreg_state, this);
}
void LoadTaggedElement::GenerateCode(MaglevAssembler* masm,
const ProcessingState& state) {
Register object = ToRegister(object_input());
Register index = ToRegister(index_input());
Register result_reg = ToRegister(result());
__ AssertNotSmi(object);
if (FLAG_debug_code) {
__ CmpObjectType(object, JS_OBJECT_TYPE, kScratchRegister);
__ Assert(above_equal, AbortReason::kUnexpectedValue);
}
__ DecompressAnyTagged(kScratchRegister,
FieldOperand(object, JSObject::kElementsOffset));
if (FLAG_debug_code) {
__ CmpObjectType(kScratchRegister, FIXED_ARRAY_TYPE, kScratchRegister);
__ Assert(equal, AbortReason::kUnexpectedValue);
// Reload since CmpObjectType clobbered the scratch register.
__ DecompressAnyTagged(kScratchRegister,
FieldOperand(object, JSObject::kElementsOffset));
}
__ AssertSmi(index);
// Zero out top bits of index reg (these were previously either zero already,
// or the cage base). This technically mutates it, but since it's a Smi, that
// doesn't matter.
__ movl(index, index);
static_assert(kSmiTagSize + kSmiShiftSize < times_tagged_size,
"Folding the Smi shift into the FixedArray entry size shift "
"only works if the shift is small");
__ DecompressAnyTagged(
result_reg,
FieldOperand(kScratchRegister, index,
static_cast<ScaleFactor>(times_tagged_size -
(kSmiTagSize + kSmiShiftSize)),
FixedArray::kHeaderSize));
}
void LoadDoubleElement::AllocateVreg(MaglevVregAllocationState* vreg_state) {
UseRegister(object_input());
UseRegister(index_input());
DefineAsRegister(vreg_state, this);
}
void LoadDoubleElement::GenerateCode(MaglevAssembler* masm,
const ProcessingState& state) {
Register object = ToRegister(object_input());
Register index = ToRegister(index_input());
DoubleRegister result_reg = ToDoubleRegister(result());
__ AssertNotSmi(object);
if (FLAG_debug_code) {
__ CmpObjectType(object, JS_OBJECT_TYPE, kScratchRegister);
__ Assert(above_equal, AbortReason::kUnexpectedValue);
}
__ DecompressAnyTagged(kScratchRegister,
FieldOperand(object, JSObject::kElementsOffset));
if (FLAG_debug_code) {
__ CmpObjectType(kScratchRegister, FIXED_DOUBLE_ARRAY_TYPE,
kScratchRegister);
__ Assert(equal, AbortReason::kUnexpectedValue);
// Reload since CmpObjectType clobbered the scratch register.
__ DecompressAnyTagged(kScratchRegister,
FieldOperand(object, JSObject::kElementsOffset));
}
__ AssertSmi(index);
// Zero out top bits of index reg (these were previously either zero already,
// or the cage base). This technically mutates it, but since it's a Smi, that
// doesn't matter.
__ movl(index, index);
static_assert(kSmiTagSize + kSmiShiftSize < times_8,
"Folding the Smi shift into the FixedArray entry size shift "
"only works if the shift is small");
__ Movsd(result_reg,
FieldOperand(kScratchRegister, index,
static_cast<ScaleFactor>(times_8 -
(kSmiTagSize + kSmiShiftSize)),
FixedDoubleArray::kHeaderSize));
}
void StoreTaggedFieldNoWriteBarrier::AllocateVreg(
MaglevVregAllocationState* vreg_state) {
UseRegister(object_input());
......
src/maglev/maglev-ir.h
View file @ 933e3d8b
......@@ -144,8 +144,6 @@ class CompactInterpreterFrameState;
V(InitialValue) \
V(LoadTaggedField) \
V(LoadDoubleField) \
V(LoadTaggedElement) \
V(LoadDoubleElement) \
V(LoadGlobal) \
V(LoadNamedGeneric) \
V(LoadNamedFromSuperGeneric) \
......@@ -194,8 +192,6 @@ class CompactInterpreterFrameState;
V(CheckSymbol) \
V(CheckString) \
V(CheckMapsWithMigration) \
V(CheckJSArrayBounds) \
V(CheckJSObjectElementsBounds) \
V(GeneratorStore) \
V(JumpLoopPrologue) \
V(StoreTaggedFieldNoWriteBarrier) \
......@@ -2646,39 +2642,6 @@ class CheckMapsWithMigration
const CheckType check_type_;
};
class CheckJSArrayBounds : public FixedInputNodeT<2, CheckJSArrayBounds> {
using Base = FixedInputNodeT<2, CheckJSArrayBounds>;
public:
explicit CheckJSArrayBounds(uint64_t bitfield) : Base(bitfield) {}
static constexpr OpProperties kProperties = OpProperties::EagerDeopt();
static constexpr int kReceiverIndex = 0;
static constexpr int kIndexIndex = 1;
Input& receiver_input() { return input(kReceiverIndex); }
Input& index_input() { return input(kIndexIndex); }
DECL_NODE_INTERFACE_WITH_EMPTY_PRINT_PARAMS()
};
class CheckJSObjectElementsBounds
: public FixedInputNodeT<2, CheckJSObjectElementsBounds> {
using Base = FixedInputNodeT<2, CheckJSObjectElementsBounds>;
public:
explicit CheckJSObjectElementsBounds(uint64_t bitfield) : Base(bitfield) {}
static constexpr OpProperties kProperties = OpProperties::EagerDeopt();
static constexpr int kReceiverIndex = 0;
static constexpr int kIndexIndex = 1;
Input& receiver_input() { return input(kReceiverIndex); }
Input& index_input() { return input(kIndexIndex); }
DECL_NODE_INTERFACE_WITH_EMPTY_PRINT_PARAMS()
};
class CheckedInternalizedString
: public FixedInputValueNodeT<1, CheckedInternalizedString> {
using Base = FixedInputValueNodeT<1, CheckedInternalizedString>;
......@@ -2773,39 +2736,6 @@ class LoadDoubleField : public FixedInputValueNodeT<1, LoadDoubleField> {
const int offset_;
};
class LoadTaggedElement : public FixedInputValueNodeT<2, LoadTaggedElement> {
using Base = FixedInputValueNodeT<2, LoadTaggedElement>;
public:
explicit LoadTaggedElement(uint64_t bitfield) : Base(bitfield) {}
static constexpr OpProperties kProperties = OpProperties::Reading();
static constexpr int kObjectIndex = 0;
static constexpr int kIndexIndex = 1;
Input& object_input() { return input(kObjectIndex); }
Input& index_input() { return input(kIndexIndex); }
DECL_NODE_INTERFACE_WITH_EMPTY_PRINT_PARAMS()
};
class LoadDoubleElement : public FixedInputValueNodeT<2, LoadDoubleElement> {
using Base = FixedInputValueNodeT<2, LoadDoubleElement>;
public:
explicit LoadDoubleElement(uint64_t bitfield) : Base(bitfield) {}
static constexpr OpProperties kProperties =
OpProperties::Reading() | OpProperties::Float64();
static constexpr int kObjectIndex = 0;
static constexpr int kIndexIndex = 1;
Input& object_input() { return input(kObjectIndex); }
Input& index_input() { return input(kIndexIndex); }
DECL_NODE_INTERFACE_WITH_EMPTY_PRINT_PARAMS()
};
class StoreTaggedFieldNoWriteBarrier
: public FixedInputNodeT<2, StoreTaggedFieldNoWriteBarrier> {
using Base = FixedInputNodeT<2, StoreTaggedFieldNoWriteBarrier>;
......
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