[runtime] Cleanup: we don't need field representation tracking flags

Also, copying hints can be removed from literals. Shallow
copying wasn't used for some time, because of the
way we treat mutable heap numbers.

Change-Id: Ieeba44a9f8e80c4183af8f4751f68dd3a542532e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3009230
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Michael Stanton <mvstanton@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75717}
This commit is contained in:
Mike Stanton 2021-07-13 16:45:55 +02:00 committed by V8 LUCI CQ
parent 66f5de1b44
commit 6071c6d8d6
9 changed files with 23 additions and 69 deletions

View File

@ -496,9 +496,8 @@ DEFINE_BOOL(jitless, V8_LITE_BOOL,
// Jitless V8 has a few implications:
DEFINE_NEG_IMPLICATION(jitless, opt)
// Field representation tracking is only used by TurboFan.
// Field type tracking is only used by TurboFan.
DEFINE_NEG_IMPLICATION(jitless, track_field_types)
DEFINE_NEG_IMPLICATION(jitless, track_heap_object_fields)
// Regexps are interpreted.
DEFINE_IMPLICATION(jitless, regexp_interpret_all)
#if ENABLE_SPARKPLUG
@ -542,16 +541,7 @@ DEFINE_BOOL(trace_pretenuring, false,
"trace pretenuring decisions of HAllocate instructions")
DEFINE_BOOL(trace_pretenuring_statistics, false,
"trace allocation site pretenuring statistics")
DEFINE_BOOL(track_fields, true, "track fields with only smi values")
DEFINE_BOOL(track_double_fields, true, "track fields with double values")
DEFINE_BOOL(track_heap_object_fields, true, "track fields with heap values")
DEFINE_BOOL(track_computed_fields, true, "track computed boilerplate fields")
DEFINE_IMPLICATION(track_double_fields, track_fields)
DEFINE_IMPLICATION(track_heap_object_fields, track_fields)
DEFINE_IMPLICATION(track_computed_fields, track_fields)
DEFINE_BOOL(track_field_types, true, "track field types")
DEFINE_IMPLICATION(track_field_types, track_fields)
DEFINE_IMPLICATION(track_field_types, track_heap_object_fields)
DEFINE_BOOL(trace_block_coverage, false,
"trace collected block coverage information")
DEFINE_BOOL(trace_protector_invalidation, false,

View File

@ -1445,9 +1445,6 @@ void AccessorAssembler::CheckFieldType(TNode<DescriptorArray> descriptors,
TNode<Word32T> representation,
TNode<Object> value, Label* bailout) {
Label r_smi(this), r_double(this), r_heapobject(this), all_fine(this);
// Ignore FLAG_track_fields etc. and always emit code for all checks,
// because this builtin is part of the snapshot and therefore should
// be flag independent.
GotoIf(Word32Equal(representation, Int32Constant(Representation::kSmi)),
&r_smi);
GotoIf(Word32Equal(representation, Int32Constant(Representation::kDouble)),

View File

@ -3706,15 +3706,10 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
.ToHandleChecked();
// done
// TODO(ishell): Consider using Representation::HeapObject() here and rely
// on the inplace update logic to take care of the case where someone ever
// stores a Smi into the done field. The logic works fine but in --jitless
// mode FLAG_track_heap_object_fields is off and the logic doesn't handle
// generalizations of HeapObject representation properly.
map = Map::CopyWithField(isolate(), map, factory->done_string(),
FieldType::Any(isolate()), NONE,
PropertyConstness::kConst,
Representation::Tagged(), INSERT_TRANSITION)
Representation::HeapObject(), INSERT_TRANSITION)
.ToHandleChecked();
native_context()->set_iterator_result_map(*map);

View File

@ -473,22 +473,17 @@ bool Object::FilterKey(PropertyFilter filter) {
}
Representation Object::OptimalRepresentation(PtrComprCageBase cage_base) const {
if (!FLAG_track_fields) return Representation::Tagged();
if (IsSmi()) {
return Representation::Smi();
}
HeapObject heap_object = HeapObject::cast(*this);
if (FLAG_track_double_fields && heap_object.IsHeapNumber(cage_base)) {
if (heap_object.IsHeapNumber(cage_base)) {
return Representation::Double();
} else if (FLAG_track_computed_fields &&
heap_object.IsUninitialized(
} else if (heap_object.IsUninitialized(
heap_object.GetReadOnlyRoots(cage_base))) {
return Representation::None();
} else if (FLAG_track_heap_object_fields) {
return Representation::HeapObject();
} else {
return Representation::Tagged();
}
return Representation::HeapObject();
}
ElementsKind Object::OptimalElementsKind(PtrComprCageBase cage_base) const {
@ -499,13 +494,13 @@ ElementsKind Object::OptimalElementsKind(PtrComprCageBase cage_base) const {
bool Object::FitsRepresentation(Representation representation,
bool allow_coercion) const {
if (FLAG_track_fields && representation.IsSmi()) {
if (representation.IsSmi()) {
return IsSmi();
} else if (FLAG_track_double_fields && representation.IsDouble()) {
} else if (representation.IsDouble()) {
return allow_coercion ? IsNumber() : IsHeapNumber();
} else if (FLAG_track_heap_object_fields && representation.IsHeapObject()) {
} else if (representation.IsHeapObject()) {
return IsHeapObject();
} else if (FLAG_track_fields && representation.IsNone()) {
} else if (representation.IsNone()) {
return false;
}
return true;

View File

@ -34,13 +34,11 @@ void PreInitializeLiteralSite(Handle<FeedbackVector> vector,
vector->SynchronizedSet(slot, Smi::FromInt(1));
}
enum DeepCopyHints { kNoHints = 0, kObjectIsShallow = 1 };
template <class ContextObject>
class JSObjectWalkVisitor {
public:
JSObjectWalkVisitor(ContextObject* site_context, DeepCopyHints hints)
: site_context_(site_context), hints_(hints) {}
explicit JSObjectWalkVisitor(ContextObject* site_context)
: site_context_(site_context) {}
V8_WARN_UNUSED_RESULT MaybeHandle<JSObject> StructureWalk(
Handle<JSObject> object);
@ -64,7 +62,6 @@ class JSObjectWalkVisitor {
private:
ContextObject* site_context_;
const DeepCopyHints hints_;
};
template <class ContextObject>
@ -72,9 +69,8 @@ MaybeHandle<JSObject> JSObjectWalkVisitor<ContextObject>::StructureWalk(
Handle<JSObject> object) {
Isolate* isolate = this->isolate();
bool copying = ContextObject::kCopying;
bool shallow = hints_ == kObjectIsShallow;
if (!shallow) {
{
StackLimitCheck check(isolate);
if (check.HasOverflowed()) {
@ -105,8 +101,6 @@ MaybeHandle<JSObject> JSObjectWalkVisitor<ContextObject>::StructureWalk(
DCHECK(copying || copy.is_identical_to(object));
if (shallow) return copy;
HandleScope scope(isolate);
// Deep copy own properties. Arrays only have 1 property "length".
@ -316,7 +310,7 @@ class AllocationSiteCreationContext : public AllocationSiteContext {
MaybeHandle<JSObject> DeepWalk(Handle<JSObject> object,
DeprecationUpdateContext* site_context) {
JSObjectWalkVisitor<DeprecationUpdateContext> v(site_context, kNoHints);
JSObjectWalkVisitor<DeprecationUpdateContext> v(site_context);
MaybeHandle<JSObject> result = v.StructureWalk(object);
Handle<JSObject> for_assert;
DCHECK(!result.ToHandle(&for_assert) || for_assert.is_identical_to(object));
@ -325,7 +319,7 @@ MaybeHandle<JSObject> DeepWalk(Handle<JSObject> object,
MaybeHandle<JSObject> DeepWalk(Handle<JSObject> object,
AllocationSiteCreationContext* site_context) {
JSObjectWalkVisitor<AllocationSiteCreationContext> v(site_context, kNoHints);
JSObjectWalkVisitor<AllocationSiteCreationContext> v(site_context);
MaybeHandle<JSObject> result = v.StructureWalk(object);
Handle<JSObject> for_assert;
DCHECK(!result.ToHandle(&for_assert) || for_assert.is_identical_to(object));
@ -333,9 +327,8 @@ MaybeHandle<JSObject> DeepWalk(Handle<JSObject> object,
}
MaybeHandle<JSObject> DeepCopy(Handle<JSObject> object,
AllocationSiteUsageContext* site_context,
DeepCopyHints hints) {
JSObjectWalkVisitor<AllocationSiteUsageContext> v(site_context, hints);
AllocationSiteUsageContext* site_context) {
JSObjectWalkVisitor<AllocationSiteUsageContext> v(site_context);
MaybeHandle<JSObject> copy = v.StructureWalk(object);
Handle<JSObject> for_assert;
DCHECK(!copy.ToHandle(&for_assert) || !for_assert.is_identical_to(object));
@ -520,26 +513,13 @@ Handle<JSObject> CreateArrayLiteral(
copied_elements_values->length(), allocation);
}
inline DeepCopyHints DecodeCopyHints(int flags) {
DeepCopyHints copy_hints =
(flags & AggregateLiteral::kIsShallow) ? kObjectIsShallow : kNoHints;
if (FLAG_track_double_fields) {
// Make sure we properly clone mutable heap numbers on 32-bit platforms.
copy_hints = kNoHints;
}
return copy_hints;
}
template <typename LiteralHelper>
MaybeHandle<JSObject> CreateLiteralWithoutAllocationSite(
Isolate* isolate, Handle<HeapObject> description, int flags) {
Handle<JSObject> literal = LiteralHelper::Create(isolate, description, flags,
AllocationType::kYoung);
DeepCopyHints copy_hints = DecodeCopyHints(flags);
if (copy_hints == kNoHints) {
DeprecationUpdateContext update_context(isolate);
RETURN_ON_EXCEPTION(isolate, DeepWalk(literal, &update_context), JSObject);
}
DeprecationUpdateContext update_context(isolate);
RETURN_ON_EXCEPTION(isolate, DeepWalk(literal, &update_context), JSObject);
return literal;
}
@ -558,8 +538,6 @@ MaybeHandle<JSObject> CreateLiteral(Isolate* isolate,
CHECK(literals_slot.ToInt() < vector->length());
Handle<Object> literal_site(vector->Get(literals_slot)->cast<Object>(),
isolate);
DeepCopyHints copy_hints = DecodeCopyHints(flags);
Handle<AllocationSite> site;
Handle<JSObject> boilerplate;
@ -596,8 +574,7 @@ MaybeHandle<JSObject> CreateLiteral(Isolate* isolate,
// Copy the existing boilerplate.
AllocationSiteUsageContext usage_context(isolate, site, enable_mementos);
usage_context.EnterNewScope();
MaybeHandle<JSObject> copy =
DeepCopy(boilerplate, &usage_context, copy_hints);
MaybeHandle<JSObject> copy = DeepCopy(boilerplate, &usage_context);
usage_context.ExitScope(site, boilerplate);
return copy;
}

View File

@ -25,7 +25,7 @@
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
// Flags: --track-fields --track-double-fields --allow-natives-syntax
// Flags: --allow-natives-syntax
// Flags: --concurrent-recompilation --block-concurrent-recompilation
// Flags: --no-always-opt --no-turbo-concurrent-get-property-access-info

View File

@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-ayle license that can be
// found in the LICENSE file.
// Flags: --allow-natives-syntax --track-fields --expose-gc
// Flags: --allow-natives-syntax --expose-gc
var global = Function('return this')();
var verbose = 0;

View File

@ -25,7 +25,7 @@
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
// Flags: --track-fields --track-double-fields --allow-natives-syntax
// Flags: --allow-natives-syntax
function smi_field() {
// Assign twice to make the field non-constant.

View File

@ -25,7 +25,7 @@
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
// Flags: --track-fields --track-double-fields --allow-natives-syntax
// Flags: --allow-natives-syntax
// Test transitions caused by changes to field representations.