From 25808bbc1578d436c300cf2ee5d6674f5e6442f8 Mon Sep 17 00:00:00 2001 From: Tobias Tebbi Date: Wed, 8 Apr 2020 12:15:50 +0200 Subject: [PATCH] [torque] remove HasIndexedField and cleanup internal class lists The two refactorings are somewhat orthogonal, but intersect at the class and instance type list generation, which is why it's easier to put them in one CL. For the removal of HasIndexedField, the removal is motivated by the fact that is no longer necessary, and that using a flag to store this kind of information is hacky. For the class list changes, this is a cleanup in that we no longer generate third-order macros, but instead normal macro lists. There is a functional change and bug-fix in that we no longer include abstract classes in lists that refer to instance types or maps. It's still somewhat broken though, so I can't test abstract internal classes yet, though. Coming in a follow-up CL. TBR=ulan@chromium.org Bug: v8:7793 Change-Id: Ided8591370570ca3810d7991f53177ca32e03048 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2108034 Commit-Queue: Tobias Tebbi Reviewed-by: Nico Hartmann Cr-Commit-Position: refs/heads/master@{#67056} --- src/codegen/code-stub-assembler.h | 6 +-- src/compiler/code-assembler.h | 4 +- src/compiler/types.cc | 4 +- src/heap/setup-heap-internal.cc | 14 +++--- src/objects/map.cc | 8 +-- src/objects/object-list-macros.h | 4 +- src/objects/objects-definitions.h | 9 ---- src/roots/roots.h | 18 +++---- src/torque/implementation-visitor.cc | 4 +- src/torque/instance-type-generator.cc | 72 ++++++++++++++++----------- src/torque/types.cc | 28 +++-------- src/torque/types.h | 8 --- tools/v8heapconst.py | 14 +++--- 13 files changed, 83 insertions(+), 110 deletions(-) diff --git a/src/codegen/code-stub-assembler.h b/src/codegen/code-stub-assembler.h index fe56765612..8c01b8afc6 100644 --- a/src/codegen/code-stub-assembler.h +++ b/src/codegen/code-stub-assembler.h @@ -53,9 +53,6 @@ enum class PrimitiveType { kBoolean, kNumber, kString, kSymbol }; V(TypedArraySpeciesProtector, typed_array_species_protector, \ TypedArraySpeciesProtector) -#define TORQUE_INTERNAL_CLASS_LIST_CSA_ADAPTER(V, NAME, Name, name) \ - V(Name##Map, name##_map, Name##Map) - #define HEAP_IMMUTABLE_IMMOVABLE_OBJECT_LIST(V) \ V(AccessorInfoMap, accessor_info_map, AccessorInfoMap) \ V(AccessorPairMap, accessor_pair_map, AccessorPairMap) \ @@ -178,8 +175,7 @@ enum class PrimitiveType { kBoolean, kNumber, kString, kSymbol }; V(uninitialized_symbol, uninitialized_symbol, UninitializedSymbol) \ V(WeakFixedArrayMap, weak_fixed_array_map, WeakFixedArrayMap) \ V(zero_string, zero_string, ZeroString) \ - TORQUE_INTERNAL_CLASS_LIST_GENERATOR(TORQUE_INTERNAL_CLASS_LIST_CSA_ADAPTER, \ - V) + TORQUE_INTERNAL_MAP_CSA_LIST(V) #define HEAP_IMMOVABLE_OBJECT_LIST(V) \ HEAP_MUTABLE_IMMOVABLE_OBJECT_LIST(V) \ diff --git a/src/compiler/code-assembler.h b/src/compiler/code-assembler.h index 3a137fdee2..2544b071e4 100644 --- a/src/compiler/code-assembler.h +++ b/src/compiler/code-assembler.h @@ -76,8 +76,8 @@ class PromiseReactionJobTask; class PromiseRejectReactionJobTask; class WasmDebugInfo; class Zone; -#define MAKE_FORWARD_DECLARATION(V, NAME, Name, name) class Name; -TORQUE_INTERNAL_CLASS_LIST_GENERATOR(MAKE_FORWARD_DECLARATION, UNUSED) +#define MAKE_FORWARD_DECLARATION(Name) class Name; +TORQUE_INTERNAL_CLASS_LIST(MAKE_FORWARD_DECLARATION) #undef MAKE_FORWARD_DECLARATION template diff --git a/src/compiler/types.cc b/src/compiler/types.cc index 907196120b..c505c50c1a 100644 --- a/src/compiler/types.cc +++ b/src/compiler/types.cc @@ -369,8 +369,8 @@ Type::bitset BitsetType::Lub(const MapRefLike& map) { case PROMISE_FULFILL_REACTION_JOB_TASK_TYPE: case PROMISE_REJECT_REACTION_JOB_TASK_TYPE: case PROMISE_RESOLVE_THENABLE_JOB_TASK_TYPE: -#define MAKE_TORQUE_CLASS_TYPE(V) case V: - TORQUE_INSTANCE_TYPES(MAKE_TORQUE_CLASS_TYPE) +#define MAKE_TORQUE_CLASS_TYPE(INSTANCE_TYPE, Name, name) case INSTANCE_TYPE: + TORQUE_INTERNAL_INSTANCE_TYPE_LIST(MAKE_TORQUE_CLASS_TYPE) #undef MAKE_TORQUE_CLASS_TYPE UNREACHABLE(); } diff --git a/src/heap/setup-heap-internal.cc b/src/heap/setup-heap-internal.cc index 8c9d8cd456..73753641a7 100644 --- a/src/heap/setup-heap-internal.cc +++ b/src/heap/setup-heap-internal.cc @@ -403,17 +403,15 @@ bool Heap::CreateInitialMaps() { ALLOCATE_VARSIZE_MAP(SMALL_ORDERED_NAME_DICTIONARY_TYPE, small_ordered_name_dictionary) -#define TORQUE_INTERNAL_CLASS_LIST_MAP_ALLOCATOR(V, NAME, Name, name) \ +#define TORQUE_ALLOCATE_MAP(NAME, Name, name) \ ALLOCATE_MAP(NAME, Name::kSize, name) - TORQUE_INTERNAL_FIXED_CLASS_LIST_GENERATOR( - TORQUE_INTERNAL_CLASS_LIST_MAP_ALLOCATOR, _); -#undef TORQUE_INTERNAL_CLASS_LIST_MAP_ALLOCATOR + TORQUE_INTERNAL_FIXED_INSTANCE_TYPE_LIST(TORQUE_ALLOCATE_MAP); +#undef TORQUE_ALLOCATE_MAP -#define TORQUE_INTERNAL_CLASS_LIST_MAP_ALLOCATOR(V, NAME, Name, name) \ +#define TORQUE_ALLOCATE_VARSIZE_MAP(NAME, Name, name) \ ALLOCATE_VARSIZE_MAP(NAME, name) - TORQUE_INTERNAL_VARSIZE_CLASS_LIST_GENERATOR( - TORQUE_INTERNAL_CLASS_LIST_MAP_ALLOCATOR, _); -#undef TORQUE_INTERNAL_CLASS_LIST_MAP_ALLOCATOR + TORQUE_INTERNAL_VARSIZE_INSTANCE_TYPE_LIST(TORQUE_ALLOCATE_VARSIZE_MAP); +#undef TORQUE_ALLOCATE_VARSIZE_MAP ALLOCATE_VARSIZE_MAP(FIXED_ARRAY_TYPE, sloppy_arguments_elements) diff --git a/src/objects/map.cc b/src/objects/map.cc index 2dc288628c..072a09717b 100644 --- a/src/objects/map.cc +++ b/src/objects/map.cc @@ -84,11 +84,11 @@ Map Map::GetInstanceTypeMap(ReadOnlyRoots roots, InstanceType type) { break; STRUCT_LIST(MAKE_CASE) #undef MAKE_CASE -#define MAKE_CASE(_, TYPE, Name, name) \ - case TYPE: \ - map = roots.name##_map(); \ +#define MAKE_CASE(TYPE, Name, name) \ + case TYPE: \ + map = roots.name##_map(); \ break; - TORQUE_INTERNAL_CLASS_LIST_GENERATOR(MAKE_CASE, _) + TORQUE_INTERNAL_INSTANCE_TYPE_LIST(MAKE_CASE) #undef MAKE_CASE default: UNREACHABLE(); diff --git a/src/objects/object-list-macros.h b/src/objects/object-list-macros.h index 11b5c034c9..12662eb805 100644 --- a/src/objects/object-list-macros.h +++ b/src/objects/object-list-macros.h @@ -7,8 +7,6 @@ #include "torque-generated/instance-types-tq.h" -#define TORQUE_INTERNAL_CLASS_NAMES_ADAPTER(V, NAME, Name, name) V(Name) - namespace v8 { namespace internal { @@ -238,7 +236,7 @@ class ZoneForwardList; V(WeakFixedArray) \ V(WeakArrayList) \ V(WeakCell) \ - TORQUE_INTERNAL_CLASS_LIST_GENERATOR(TORQUE_INTERNAL_CLASS_NAMES_ADAPTER, V) + TORQUE_INTERNAL_CLASS_LIST(V) #ifdef V8_INTL_SUPPORT #define HEAP_OBJECT_ORDINARY_TYPE_LIST(V) \ diff --git a/src/objects/objects-definitions.h b/src/objects/objects-definitions.h index b97a8f47fc..8a990cbc63 100644 --- a/src/objects/objects-definitions.h +++ b/src/objects/objects-definitions.h @@ -175,15 +175,6 @@ namespace internal { // Produces (Map, struct_name_map, StructNameMap) entries #define STRUCT_MAPS_LIST(V) STRUCT_LIST_GENERATOR(STRUCT_MAPS_LIST_ADAPTER, V) -// Adapts one STRUCT_LIST_GENERATOR entry to the STRUCT_LIST entry -#define TORQUE_INTERNAL_CLASS_LIST_MAPS_ADAPTER(V, NAME, Name, name) \ - V(Map, name##_map, Name##Map) - -// Produces (NAME, Name, name) entries. -#define TORQUE_INTERNAL_CLASS_MAPS_LIST(V) \ - TORQUE_INTERNAL_CLASS_LIST_GENERATOR( \ - TORQUE_INTERNAL_CLASS_LIST_MAPS_ADAPTER, V) - // // The following macros define list of allocation size objects and list of // their maps. diff --git a/src/roots/roots.h b/src/roots/roots.h index cf84ebf40b..421e7ba0b3 100644 --- a/src/roots/roots.h +++ b/src/roots/roots.h @@ -302,15 +302,15 @@ class Symbol; #define ACCESSOR_INFO_ROOT_LIST(V) \ ACCESSOR_INFO_LIST_GENERATOR(ACCESSOR_INFO_ROOT_LIST_ADAPTER, V) -#define READ_ONLY_ROOT_LIST(V) \ - STRONG_READ_ONLY_ROOT_LIST(V) \ - INTERNALIZED_STRING_ROOT_LIST(V) \ - PRIVATE_SYMBOL_ROOT_LIST(V) \ - PUBLIC_SYMBOL_ROOT_LIST(V) \ - WELL_KNOWN_SYMBOL_ROOT_LIST(V) \ - STRUCT_MAPS_LIST(V) \ - TORQUE_INTERNAL_CLASS_MAPS_LIST(V) \ - ALLOCATION_SITE_MAPS_LIST(V) \ +#define READ_ONLY_ROOT_LIST(V) \ + STRONG_READ_ONLY_ROOT_LIST(V) \ + INTERNALIZED_STRING_ROOT_LIST(V) \ + PRIVATE_SYMBOL_ROOT_LIST(V) \ + PUBLIC_SYMBOL_ROOT_LIST(V) \ + WELL_KNOWN_SYMBOL_ROOT_LIST(V) \ + STRUCT_MAPS_LIST(V) \ + TORQUE_INTERNAL_MAP_ROOT_LIST(V) \ + ALLOCATION_SITE_MAPS_LIST(V) \ DATA_HANDLER_MAPS_LIST(V) #define MUTABLE_ROOT_LIST(V) \ diff --git a/src/torque/implementation-visitor.cc b/src/torque/implementation-visitor.cc index badd92a207..fe96bd26a3 100644 --- a/src/torque/implementation-visitor.cc +++ b/src/torque/implementation-visitor.cc @@ -3112,10 +3112,10 @@ class FieldOffsetsGenerator { // In the presence of indexed fields, we already emitted kHeaderSize before // the indexed field. - if (!type_->IsShape() && !type_->HasIndexedField()) { + if (!type_->IsShape() && !header_size_emitted_) { WriteMarker("kHeaderSize"); } - if (type_->HasStaticSize()) { + if (!type_->IsAbstract() && type_->HasStaticSize()) { WriteMarker("kSize"); } } diff --git a/src/torque/instance-type-generator.cc b/src/torque/instance-type-generator.cc index a06c984629..622e8f738e 100644 --- a/src/torque/instance-type-generator.cc +++ b/src/torque/instance-type-generator.cc @@ -433,42 +433,54 @@ void ImplementationVisitor::GenerateInstanceTypes( header << only_declared_range_instance_types.str(); header << "\n"; - header << "// Instance types for non-extern Torque classes.\n"; - header << "#define TORQUE_INSTANCE_TYPES(V) \\\n"; - for (const ClassType* type : TypeOracle::GetClasses()) { - if (type->IsExtern()) continue; - std::string type_name = - CapifyStringWithUnderscores(type->name()) + "_TYPE"; - header << " V(" << type_name << ") \\\n"; - } - header << "\n"; + std::stringstream torque_internal_class_list; + std::stringstream torque_internal_varsize_instance_type_list; + std::stringstream torque_internal_fixed_instance_type_list; + std::stringstream torque_internal_map_csa_list; + std::stringstream torque_internal_map_root_list; - header << "// Map list macros for non-extern Torque classes.\n"; - header << "#define TORQUE_INTERNAL_VARSIZE_CLASS_LIST_GENERATOR(V, _) \\\n"; for (const ClassType* type : TypeOracle::GetClasses()) { - if (type->IsExtern()) continue; - if (!type->HasIndexedField()) continue; - std::string type_name = + std::string upper_case_name = type->name(); + std::string lower_case_name = SnakeifyString(type->name()); + std::string instance_type_name = CapifyStringWithUnderscores(type->name()) + "_TYPE"; - std::string variable_name = SnakeifyString(type->name()); - header << " V(_, " << type_name << ", " << type->name() << ", " - << variable_name << ") \\\n"; - } - header << "\n"; - header << "#define TORQUE_INTERNAL_FIXED_CLASS_LIST_GENERATOR(V, _) \\\n"; - for (const ClassType* type : TypeOracle::GetClasses()) { + if (type->IsExtern()) continue; - if (type->HasIndexedField()) continue; - std::string type_name = - CapifyStringWithUnderscores(type->name()) + "_TYPE"; - std::string variable_name = SnakeifyString(type->name()); - header << " V(_, " << type_name << ", " << type->name() << ", " - << variable_name << ") \\\n"; + torque_internal_class_list << " V(" << upper_case_name << ") \\\n"; + + if (type->IsAbstract()) continue; + torque_internal_map_csa_list << " V(" << upper_case_name << "Map, " + << lower_case_name << "_map, " + << upper_case_name << "Map) \\\n"; + torque_internal_map_root_list << " V(Map, " << lower_case_name + << "_map, " << upper_case_name + << "Map) \\\n"; + std::stringstream& list = + type->HasStaticSize() ? torque_internal_fixed_instance_type_list + : torque_internal_varsize_instance_type_list; + list << " V(" << instance_type_name << ", " << upper_case_name << ", " + << lower_case_name << ") \\\n"; } + + header << "// Non-extern Torque classes.\n"; + header << "#define TORQUE_INTERNAL_CLASS_LIST(V) \\\n"; + header << torque_internal_class_list.str(); header << "\n"; - header << "#define TORQUE_INTERNAL_CLASS_LIST_GENERATOR(V, _) \\\n"; - header << " TORQUE_INTERNAL_VARSIZE_CLASS_LIST_GENERATOR(V, _) \\\n"; - header << " TORQUE_INTERNAL_FIXED_CLASS_LIST_GENERATOR(V, _)\n"; + header << "#define TORQUE_INTERNAL_VARSIZE_INSTANCE_TYPE_LIST(V) \\\n"; + header << torque_internal_varsize_instance_type_list.str(); + header << "\n"; + header << "#define TORQUE_INTERNAL_FIXED_INSTANCE_TYPE_LIST(V) \\\n"; + header << torque_internal_fixed_instance_type_list.str(); + header << "\n"; + header << "#define TORQUE_INTERNAL_INSTANCE_TYPE_LIST(V) \\\n"; + header << " TORQUE_INTERNAL_VARSIZE_INSTANCE_TYPE_LIST(V) \\\n"; + header << " TORQUE_INTERNAL_FIXED_INSTANCE_TYPE_LIST(V) \\\n"; + header << "\n"; + header << "#define TORQUE_INTERNAL_MAP_CSA_LIST(V) \\\n"; + header << torque_internal_map_csa_list.str(); + header << "\n"; + header << "#define TORQUE_INTERNAL_MAP_ROOT_LIST(V) \\\n"; + header << torque_internal_map_root_list.str(); header << "\n"; } std::string output_header_path = output_directory + "/" + file_name; diff --git a/src/torque/types.cc b/src/torque/types.cc index e08716c98a..c5138a7d79 100644 --- a/src/torque/types.cc +++ b/src/torque/types.cc @@ -471,25 +471,16 @@ void StructType::Finalize() const { CheckForDuplicateFields(); } -constexpr ClassFlags ClassType::kInternalFlags; - ClassType::ClassType(const Type* parent, Namespace* nspace, const std::string& name, ClassFlags flags, const std::string& generates, const ClassDeclaration* decl, const TypeAlias* alias) : AggregateType(Kind::kClassType, parent, nspace, name), size_(ResidueClass::Unknown()), - flags_(flags & ~(kInternalFlags)), + flags_(flags), generates_(generates), decl_(decl), - alias_(alias) { - DCHECK_EQ(flags & kInternalFlags, 0); -} - -bool ClassType::HasIndexedField() const { - if (!is_finalized_) Finalize(); - return flags_ & ClassFlag::kHasIndexedField; -} + alias_(alias) {} std::string ClassType::GetGeneratedTNodeTypeNameImpl() const { return generates_; @@ -510,11 +501,6 @@ void ClassType::Finalize() const { if (is_finalized_) return; CurrentScope::Scope scope_activator(alias_->ParentScope()); CurrentSourcePosition::Scope position_activator(decl_->pos); - if (parent()) { - if (const ClassType* super_class = ClassType::DynamicCast(parent())) { - if (super_class->HasIndexedField()) flags_ |= ClassFlag::kHasIndexedField; - } - } TypeVisitor::VisitClassFieldsAndMethods(const_cast(this), this->decl_); is_finalized_ = true; @@ -620,11 +606,11 @@ void ClassType::GenerateAccessors() { } bool ClassType::HasStaticSize() const { - if (IsShape()) return true; - if (IsSubtypeOf(TypeOracle::GetJSObjectType())) return false; - if (IsAbstract()) return false; - if (HasIndexedField()) return false; - return true; + // Abstract classes don't have instances directly, so asking this question + // doesn't make sense. + DCHECK(!IsAbstract()); + if (IsSubtypeOf(TypeOracle::GetJSObjectType()) && !IsShape()) return false; + return size().SingleValue().has_value(); } void PrintSignature(std::ostream& os, const Signature& sig, bool with_names) { diff --git a/src/torque/types.h b/src/torque/types.h index b60879ce85..3c15bc82f0 100644 --- a/src/torque/types.h +++ b/src/torque/types.h @@ -517,8 +517,6 @@ class AggregateType : public Type { virtual void Finalize() const = 0; - virtual bool HasIndexedField() const { return false; } - void SetFields(std::vector fields) { fields_ = std::move(fields); } const std::vector& fields() const { if (!is_finalized_) Finalize(); @@ -609,8 +607,6 @@ class TypeAlias; class ClassType final : public AggregateType { public: - static constexpr ClassFlags kInternalFlags = ClassFlag::kHasIndexedField; - DECLARE_TYPE_BOILERPLATE(ClassType) std::string ToExplicitString() const override; std::string GetGeneratedTypeNameImpl() const override; @@ -639,7 +635,6 @@ class ClassType final : public AggregateType { bool ShouldExport() const { return flags_ & ClassFlag::kExport; } bool IsShape() const { return flags_ & ClassFlag::kIsShape; } bool HasStaticSize() const; - bool HasIndexedField() const override; size_t header_size() const { if (!is_finalized_) Finalize(); return header_size_; @@ -655,9 +650,6 @@ class ClassType final : public AggregateType { void GenerateAccessors(); bool AllowInstantiation() const; const Field& RegisterField(Field field) override { - if (field.index) { - flags_ |= ClassFlag::kHasIndexedField; - } return AggregateType::RegisterField(field); } void Finalize() const override; diff --git a/tools/v8heapconst.py b/tools/v8heapconst.py index 7e9cfd2eb3..7c87b993da 100644 --- a/tools/v8heapconst.py +++ b/tools/v8heapconst.py @@ -328,13 +328,13 @@ KNOWN_MAPS = { ("read_only_space", 0x0385d): (114, "WasmIndirectFunctionTableMap"), ("read_only_space", 0x03885): (115, "WasmJSFunctionDataMap"), ("read_only_space", 0x038ad): (116, "WasmValueMap"), - ("read_only_space", 0x038d5): (135, "InternalClassWithSmiElementsMap"), - ("read_only_space", 0x038fd): (166, "InternalClassWithStructElementsMap"), - ("read_only_space", 0x03925): (165, "InternalClassMap"), - ("read_only_space", 0x0394d): (173, "SmiPairMap"), - ("read_only_space", 0x03975): (172, "SmiBoxMap"), - ("read_only_space", 0x0399d): (68, "ExportedSubClassBaseMap"), - ("read_only_space", 0x039c5): (69, "ExportedSubClassMap"), + ("read_only_space", 0x038d5): (165, "InternalClassMap"), + ("read_only_space", 0x038fd): (173, "SmiPairMap"), + ("read_only_space", 0x03925): (172, "SmiBoxMap"), + ("read_only_space", 0x0394d): (68, "ExportedSubClassBaseMap"), + ("read_only_space", 0x03975): (69, "ExportedSubClassMap"), + ("read_only_space", 0x0399d): (135, "InternalClassWithSmiElementsMap"), + ("read_only_space", 0x039c5): (166, "InternalClassWithStructElementsMap"), ("read_only_space", 0x039ed): (174, "SortStateMap"), ("read_only_space", 0x03a15): (85, "AllocationSiteWithWeakNextMap"), ("read_only_space", 0x03a3d): (85, "AllocationSiteWithoutWeakNextMap"),