AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[compiler] Perform accessors atomically only if concurrent marking is on

Browse Source 
From the concurrent compiler's perspective, we can perform those
read/writes non-atomically and have wider TSAN coverage. The concurrent
marker, however, needs them to be atomic.

Bug: v8:7790
Change-Id: I96897f4f6237c90da018ec89be838aae894c24bc
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2817538
Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73935}
This commit is contained in:
Santiago Aboy Solanes 2021-04-13 14:13:54 +01:00 committed by Commit Bot
parent db8ef77a4e
commit 53f0698ddd
4 changed files with 48 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/heap/factory.cc
View File

@ -1572,9 +1572,7 @@ Map Factory::InitializeMap(Map map, InstanceType type, int instance_size,
map.SetInstanceDescriptors(isolate(), *empty_descriptor_array(), 0);
// Must be called only after |instance_type| and |instance_size| are set.
map.set_visitor_id(Map::GetVisitorId(map));
// TODO(solanes, v8:7790, v8:11353): set_relaxed_bit_field could be an atomic
// set if TSAN could see the transitions happening in StoreIC.
map.set_relaxed_bit_field(0);
map.set_bit_field(0);
map.set_bit_field2(Map::Bits2::NewTargetIsBaseBit::encode(true));
int bit_field3 =
Map::Bits3::EnumLengthBits::encode(kInvalidEnumCacheSentinel) |

48
src/objects/map-inl.h
View File

@ -98,10 +98,10 @@ BIT_FIELD_ACCESSORS(Map, relaxed_bit_field3, is_in_retained_map_list,
Map::Bits3::IsInRetainedMapListBit)
BIT_FIELD_ACCESSORS(Map, release_acquire_bit_field3, is_prototype_map,
Map::Bits3::IsPrototypeMapBit)
BIT_FIELD_ACCESSORS(Map, relaxed_bit_field3, is_migration_target,
Map::Bits3::IsMigrationTargetBit)
BIT_FIELD_ACCESSORS(Map, relaxed_bit_field3, is_extensible,
Map::Bits3::IsExtensibleBit)
BIT_FIELD_ACCESSORS2(Map, relaxed_bit_field3, bit_field3, is_migration_target,
Map::Bits3::IsMigrationTargetBit)
BIT_FIELD_ACCESSORS2(Map, relaxed_bit_field3, bit_field3, is_extensible,
Map::Bits3::IsExtensibleBit)
BIT_FIELD_ACCESSORS(Map, bit_field3, may_have_interesting_symbols,
Map::Bits3::MayHaveInterestingSymbolsBit)
BIT_FIELD_ACCESSORS(Map, relaxed_bit_field3, construction_counter,
@ -324,15 +324,23 @@ Handle<Map> Map::AddMissingTransitionsForTesting(
return AddMissingTransitions(isolate, split_map, descriptors);
}
// TODO(solanes, v8:7790, v8:11353): Make the instance_type accessors non-atomic
// when TSAN sees the map's store synchronization.
InstanceType Map::instance_type() const {
return static_cast<InstanceType>(
RELAXED_READ_UINT16_FIELD(*this, kInstanceTypeOffset));
if (V8_CONCURRENT_MARKING_BOOL) {
// TODO(solanes, v8:7790, v8:11353): Make this and the setter non-atomic
// when TSAN sees the map's store synchronization.
return static_cast<InstanceType>(
RELAXED_READ_UINT16_FIELD(*this, kInstanceTypeOffset));
} else {
return static_cast<InstanceType>(ReadField<uint16_t>(kInstanceTypeOffset));
}
}
void Map::set_instance_type(InstanceType value) {
RELAXED_WRITE_UINT16_FIELD(*this, kInstanceTypeOffset, value);
if (V8_CONCURRENT_MARKING_BOOL) {
RELAXED_WRITE_UINT16_FIELD(*this, kInstanceTypeOffset, value);
} else {
WriteField<uint16_t>(kInstanceTypeOffset, value);
}
}
int Map::UnusedPropertyFields() const {
@ -457,7 +465,13 @@ void Map::AccountAddedOutOfObjectPropertyField(int unused_in_property_array) {
byte Map::bit_field() const { return ReadField<byte>(kBitFieldOffset); }
void Map::set_bit_field(byte value) {
WriteField<byte>(kBitFieldOffset, value);
if (V8_CONCURRENT_MARKING_BOOL) {
// TODO(solanes, v8:7790, v8:11353): Make this non-atomic when TSAN sees the
// map's store synchronization.
set_relaxed_bit_field(value);
} else {
WriteField<byte>(kBitFieldOffset, value);
}
}
byte Map::relaxed_bit_field() const {
@ -475,11 +489,21 @@ void Map::set_bit_field2(byte value) {
}
uint32_t Map::bit_field3() const {
return ReadField<uint32_t>(kBitField3Offset);
if (V8_CONCURRENT_MARKING_BOOL) {
// TODO(solanes, v8:7790, v8:11353): Make this and the setter non-atomic
// when TSAN sees the map's store synchronization.
return relaxed_bit_field3();
} else {
return ReadField<uint32_t>(kBitField3Offset);
}
}
void Map::set_bit_field3(uint32_t value) {
WriteField<uint32_t>(kBitField3Offset, value);
if (V8_CONCURRENT_MARKING_BOOL) {
set_relaxed_bit_field3(value);
} else {
WriteField<uint32_t>(kBitField3Offset, value);
}
}
uint32_t Map::relaxed_bit_field3() const {

6
src/objects/map.cc
View File

@ -1210,9 +1210,7 @@ Handle<Map> Map::RawCopy(Isolate* isolate, Handle<Map> map, int instance_size,
Handle<HeapObject> prototype(map->prototype(), isolate);
Map::SetPrototype(isolate, result, prototype);
result->set_constructor_or_back_pointer(map->GetConstructor());
// TODO(solanes, v8:7790, v8:11353): set_relaxed_bit_field could be an atomic
// set if TSAN could see the transitions happening in StoreIC.
result->set_relaxed_bit_field(map->bit_field());
result->set_bit_field(map->bit_field());
result->set_bit_field2(map->bit_field2());
int new_bit_field3 = map->bit_field3();
new_bit_field3 = Bits3::OwnsDescriptorsBit::update(new_bit_field3, true);
@ -1225,7 +1223,7 @@ Handle<Map> Map::RawCopy(Isolate* isolate, Handle<Map> map, int instance_size,
new_bit_field3 = Bits3::IsUnstableBit::update(new_bit_field3, false);
}
// Same as bit_field comment above.
result->set_relaxed_bit_field3(new_bit_field3);
result->set_bit_field3(new_bit_field3);
result->clear_padding();
return result;
}

9
src/objects/map.h
View File

@ -244,7 +244,12 @@ class Map : public HeapObject {
//
// Bit field.
//
// The setter in this pair calls the relaxed setter if concurrent marking is
// on, or performs the write non-atomically if it's off. The read is always
// non-atomically. This is done to have wider TSAN coverage on the cases where
// it's possible.
DECL_PRIMITIVE_ACCESSORS(bit_field, byte)
// Atomic accessors, used for allowlisting legitimate concurrent accesses.
DECL_PRIMITIVE_ACCESSORS(relaxed_bit_field, byte)
@ -266,7 +271,11 @@ class Map : public HeapObject {
//
// Bit field 3.
//
// {bit_field3} calls the relaxed accessors if concurrent marking is on, or
// performs the read/write non-atomically if it's off. This is done to have
// wider TSAN coverage on the cases where it's possible.
DECL_PRIMITIVE_ACCESSORS(bit_field3, uint32_t)
DECL_PRIMITIVE_ACCESSORS(relaxed_bit_field3, uint32_t)
DECL_PRIMITIVE_ACCESSORS(release_acquire_bit_field3, uint32_t)