Commit 785d701f authored by Santiago Aboy Solanes's avatar Santiago Aboy Solanes Committed by Commit Bot

Reland "[compiler] Replace JSDataView with direct reads"

This is a reland of b5f37051

Got reverted since it was breaking the bots
(https://bugs.chromium.org/p/v8/issues/detail?id=10918)

The solution is to keep the JSDataView class as kSerialized but change
its method to do a direct heap access. In this way, its map it will
still be serialized (which was the cause of the bot failure).

In order to keep incrementally skipping serialization, we can introduce
new macros that allow a per-method skip of serialization rather than
per-class.

Original change's description:
> [compiler] Replace JSDataView with direct reads
>
> Bug: v8:7790
> Change-Id: Id01c2e4359aa4294816ffe14c08a586a9b9b10c2
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2404768
> Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
> Reviewed-by: Georg Neis <neis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#69904}

Bug: v8:7790, v8:10918
Change-Id: Ifdfe504272369e7cc1332fe53992739f9d0be385
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2413258Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69949}
parent 71ab9b78
......@@ -7332,11 +7332,11 @@ Reduction JSCallReducer::ReduceDataViewAccess(Node* node, DataViewAccess access,
// We only deal with DataViews here whose [[ByteLength]] is at least
// {element_size}, as for all other DataViews it'll be out-of-bounds.
JSDataViewRef dataview = m.Ref(broker()).AsJSDataView();
if (dataview.byte_length() < element_size) return NoChange();
size_t length = dataview.byte_length();
if (length < element_size) return NoChange();
// Check that the {offset} is within range of the {byte_length}.
Node* byte_length =
jsgraph()->Constant(dataview.byte_length() - (element_size - 1));
// Check that the {offset} is within range of the {length}.
Node* byte_length = jsgraph()->Constant(length - (element_size - 1));
offset = effect = graph()->NewNode(simplified()->CheckBounds(p.feedback()),
offset, byte_length, effect, control);
} else {
......
......@@ -81,10 +81,12 @@ enum ObjectDataKind {
class AllowHandleAllocationIfNeeded {
public:
explicit AllowHandleAllocationIfNeeded(ObjectDataKind kind,
JSHeapBroker::BrokerMode mode) {
JSHeapBroker::BrokerMode mode,
bool direct_heap_access = false) {
DCHECK_IMPLIES(mode == JSHeapBroker::BrokerMode::kSerialized,
kind == kUnserializedReadOnlyHeapObject ||
kind == kNeverSerializedHeapObject);
kind == kNeverSerializedHeapObject ||
(direct_heap_access && kind == kSerializedHeapObject));
if (kind == kUnserializedHeapObject) maybe_allow_handle_.emplace();
}
......@@ -95,11 +97,13 @@ class AllowHandleAllocationIfNeeded {
class AllowHandleDereferenceIfNeeded {
public:
explicit AllowHandleDereferenceIfNeeded(ObjectDataKind kind,
JSHeapBroker::BrokerMode mode)
JSHeapBroker::BrokerMode mode,
bool direct_heap_access = false)
: AllowHandleDereferenceIfNeeded(kind) {
DCHECK_IMPLIES(mode == JSHeapBroker::BrokerMode::kSerialized,
kind == kUnserializedReadOnlyHeapObject ||
kind == kNeverSerializedHeapObject);
kind == kNeverSerializedHeapObject ||
(direct_heap_access && kind == kSerializedHeapObject));
}
explicit AllowHandleDereferenceIfNeeded(ObjectDataKind kind) {
......@@ -3330,6 +3334,27 @@ int BytecodeArrayRef::handler_table_size() const {
return BitField::decode(ObjectRef::data()->As##holder()->field()); \
}
// Like IF_ACCESS_FROM_HEAP_C but we also allow direct heap access for
// kSerialized only for methods that we identified to be safe.
#define IF_ACCESS_FROM_HEAP_WITH_FLAG_C(name) \
if (data_->should_access_heap() || FLAG_turbo_direct_heap_access) { \
AllowHandleAllocationIfNeeded handle_allocation( \
data_->kind(), broker()->mode(), FLAG_turbo_direct_heap_access); \
AllowHandleDereferenceIfNeeded allow_handle_dereference( \
data_->kind(), broker()->mode(), FLAG_turbo_direct_heap_access); \
return object()->name(); \
}
// Like BIMODAL_ACCESSOR_C except that we force a direct heap access if
// FLAG_turbo_direct_heap_access is true (even for kSerialized). This is because
// we identified the method to be safe to use direct heap access, but the
// holder##Data class still needs to be serialized.
#define BIMODAL_ACCESSOR_WITH_FLAG_C(holder, result, name) \
result holder##Ref::name() const { \
IF_ACCESS_FROM_HEAP_WITH_FLAG_C(name); \
return ObjectRef::data()->As##holder()->name(); \
}
BIMODAL_ACCESSOR(AllocationSite, Object, nested_site)
BIMODAL_ACCESSOR_C(AllocationSite, bool, CanInlineCall)
BIMODAL_ACCESSOR_C(AllocationSite, bool, PointsToLiteral)
......@@ -3357,7 +3382,7 @@ BIMODAL_ACCESSOR(JSBoundFunction, JSReceiver, bound_target_function)
BIMODAL_ACCESSOR(JSBoundFunction, Object, bound_this)
BIMODAL_ACCESSOR(JSBoundFunction, FixedArray, bound_arguments)
BIMODAL_ACCESSOR_C(JSDataView, size_t, byte_length)
BIMODAL_ACCESSOR_WITH_FLAG_C(JSDataView, size_t, byte_length)
BIMODAL_ACCESSOR_C(JSFunction, bool, has_feedback_vector)
BIMODAL_ACCESSOR_C(JSFunction, bool, has_initial_map)
......
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