llvmbot wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

<details>
<summary>Changes</summary>

Records have Program-lifetime, but we used to allocate their fields, bases and 
virtual bases using llvm::SmallVector. However, after creating a Record, it 
doesn't change and we know all the number of elements beforehand.

So, allocate the lists via the BumpPtrAllocator we already have in Program. 
This results in slight compile-time improvements:

https://llvm-compile-time-tracker.com/compare.php?from=3a3d55705b0f69f3ef5bed0b0211e7c884001842&amp;to=e0fede973116c4d43e9883586c737c3d0bb99c3e&amp;stat=instructions:u

Unfortunately, we still have the three DenseMaps in Record, so they still 
heap-allocate memory on their own.

---
Full diff: https://github.com/llvm/llvm-project/pull/147909.diff


3 Files Affected:

- (modified) clang/lib/AST/ByteCode/Program.cpp (+25-8) 
- (modified) clang/lib/AST/ByteCode/Record.cpp (+12-9) 
- (modified) clang/lib/AST/ByteCode/Record.h (+20-24) 


``````````diff
diff --git a/clang/lib/AST/ByteCode/Program.cpp 
b/clang/lib/AST/ByteCode/Program.cpp
index 5ac0f59f32d4e..de9008f0512e6 100644
--- a/clang/lib/AST/ByteCode/Program.cpp
+++ b/clang/lib/AST/ByteCode/Program.cpp
@@ -315,9 +315,17 @@ Record *Program::getOrCreateRecord(const RecordDecl *RD) {
   };
 
   // Reserve space for base classes.
-  Record::BaseList Bases;
-  Record::VirtualBaseList VirtBases;
+  Record::Base *Bases = nullptr;
+  Record::Base *VBases = nullptr;
+  unsigned NumRecordBases = 0;
+  unsigned NumRecordVBases = 0;
   if (const auto *CD = dyn_cast<CXXRecordDecl>(RD)) {
+    unsigned NumBases = CD->getNumBases();
+    unsigned NumVBases = CD->getNumVBases();
+
+    Bases = new (*this) Record::Base[NumBases];
+    VBases = new (*this) Record::Base[NumVBases];
+
     for (const CXXBaseSpecifier &Spec : CD->bases()) {
       if (Spec.isVirtual())
         continue;
@@ -334,9 +342,11 @@ Record *Program::getOrCreateRecord(const RecordDecl *RD) {
         return nullptr;
 
       BaseSize += align(sizeof(InlineDescriptor));
-      Bases.push_back({BD, BaseSize, Desc, BR});
+      new (&Bases[NumRecordBases]) Record::Base{BD, BaseSize, Desc, BR};
       BaseSize += align(BR->getSize());
+      ++NumRecordBases;
     }
+    assert(NumRecordBases <= NumBases);
 
     for (const CXXBaseSpecifier &Spec : CD->vbases()) {
       const auto *RT = Spec.getType()->getAs<RecordType>();
@@ -351,13 +361,17 @@ Record *Program::getOrCreateRecord(const RecordDecl *RD) {
         return nullptr;
 
       VirtSize += align(sizeof(InlineDescriptor));
-      VirtBases.push_back({BD, VirtSize, Desc, BR});
+      new (&VBases[NumRecordVBases]) Record::Base{BD, VirtSize, Desc, BR};
       VirtSize += align(BR->getSize());
+      ++NumRecordVBases;
     }
+    assert(NumRecordVBases <= NumVBases);
   }
 
   // Reserve space for fields.
-  Record::FieldList Fields;
+  unsigned NumFields = std::distance(RD->field_begin(), RD->field_end());
+  unsigned NumRecordFields = 0;
+  Record::Field *Fields = new (*this) Record::Field[NumFields];
   for (const FieldDecl *FD : RD->fields()) {
     FD = FD->getFirstDecl();
     // Note that we DO create fields and descriptors
@@ -382,12 +396,15 @@ Record *Program::getOrCreateRecord(const RecordDecl *RD) {
     }
     if (!Desc)
       return nullptr;
-    Fields.push_back({FD, BaseSize, Desc});
+
+    new (&Fields[NumRecordFields]) Record::Field{FD, BaseSize, Desc};
     BaseSize += align(Desc->getAllocSize());
+    ++NumRecordFields;
   }
 
-  Record *R = new (Allocator) Record(RD, std::move(Bases), std::move(Fields),
-                                     std::move(VirtBases), VirtSize, BaseSize);
+  Record *R =
+      new (Allocator) Record(RD, Bases, NumRecordBases, Fields, 
NumRecordFields,
+                             VBases, NumRecordVBases, VirtSize, BaseSize);
   Records[RD] = R;
   return R;
 }
diff --git a/clang/lib/AST/ByteCode/Record.cpp 
b/clang/lib/AST/ByteCode/Record.cpp
index 1d4ac7103cb76..e40fb63914b61 100644
--- a/clang/lib/AST/ByteCode/Record.cpp
+++ b/clang/lib/AST/ByteCode/Record.cpp
@@ -12,20 +12,23 @@
 using namespace clang;
 using namespace clang::interp;
 
-Record::Record(const RecordDecl *Decl, BaseList &&SrcBases,
-               FieldList &&SrcFields, VirtualBaseList &&SrcVirtualBases,
-               unsigned VirtualSize, unsigned BaseSize)
-    : Decl(Decl), Bases(std::move(SrcBases)), Fields(std::move(SrcFields)),
+Record::Record(const RecordDecl *Decl, const Base *SrcBases, unsigned NumBases,
+               const Field *Fields, unsigned NumFields, Base *VBases,
+               unsigned NumVBases, unsigned VirtualSize, unsigned BaseSize)
+    : Decl(Decl), Bases(SrcBases), NumBases(NumBases), Fields(Fields),
+      NumFields(NumFields), VBases(VBases), NumVBases(NumVBases),
       BaseSize(BaseSize), VirtualSize(VirtualSize), IsUnion(Decl->isUnion()),
       IsAnonymousUnion(IsUnion && Decl->isAnonymousStructOrUnion()) {
-  for (Base &V : SrcVirtualBases)
-    VirtualBases.push_back({V.Decl, V.Offset + BaseSize, V.Desc, V.R});
 
-  for (Base &B : Bases)
+  for (unsigned I = 0; I != NumVBases; ++I) {
+    VBases[I].Offset += BaseSize;
+  }
+
+  for (const Base &B : bases())
     BaseMap[B.Decl] = &B;
-  for (Field &F : Fields)
+  for (const Field &F : fields())
     FieldMap[F.Decl] = &F;
-  for (Base &V : VirtualBases)
+  for (const Base &V : virtual_bases())
     VirtualBaseMap[V.Decl] = &V;
 }
 
diff --git a/clang/lib/AST/ByteCode/Record.h b/clang/lib/AST/ByteCode/Record.h
index 93ca43046180a..2282bf060177e 100644
--- a/clang/lib/AST/ByteCode/Record.h
+++ b/clang/lib/AST/ByteCode/Record.h
@@ -41,13 +41,6 @@ class Record final {
     const Record *R;
   };
 
-  /// Mapping from identifiers to field descriptors.
-  using FieldList = llvm::SmallVector<Field, 8>;
-  /// Mapping from identifiers to base classes.
-  using BaseList = llvm::SmallVector<Base, 8>;
-  /// List of virtual base classes.
-  using VirtualBaseList = llvm::SmallVector<Base, 2>;
-
 public:
   /// Returns the underlying declaration.
   const RecordDecl *getDecl() const { return Decl; }
@@ -76,32 +69,32 @@ class Record final {
     return nullptr;
   }
 
-  using const_field_iter = FieldList::const_iterator;
+  using const_field_iter = ArrayRef<Field>::const_iterator;
   llvm::iterator_range<const_field_iter> fields() const {
-    return llvm::make_range(Fields.begin(), Fields.end());
+    return llvm::make_range(Fields, Fields + NumFields);
   }
 
-  unsigned getNumFields() const { return Fields.size(); }
+  unsigned getNumFields() const { return NumFields; }
   const Field *getField(unsigned I) const { return &Fields[I]; }
 
-  using const_base_iter = BaseList::const_iterator;
+  using const_base_iter = ArrayRef<Base>::const_iterator;
   llvm::iterator_range<const_base_iter> bases() const {
-    return llvm::make_range(Bases.begin(), Bases.end());
+    return llvm::make_range(Bases, Bases + NumBases);
   }
 
-  unsigned getNumBases() const { return Bases.size(); }
+  unsigned getNumBases() const { return NumBases; }
   const Base *getBase(unsigned I) const {
     assert(I < getNumBases());
     return &Bases[I];
   }
 
-  using const_virtual_iter = VirtualBaseList::const_iterator;
+  using const_virtual_iter = ArrayRef<Base>::const_iterator;
   llvm::iterator_range<const_virtual_iter> virtual_bases() const {
-    return llvm::make_range(VirtualBases.begin(), VirtualBases.end());
+    return llvm::make_range(VBases, VBases + NumVBases);
   }
 
-  unsigned getNumVirtualBases() const { return VirtualBases.size(); }
-  const Base *getVirtualBase(unsigned I) const { return &VirtualBases[I]; }
+  unsigned getNumVirtualBases() const { return NumVBases; }
+  const Base *getVirtualBase(unsigned I) const { return &VBases[I]; }
 
   void dump(llvm::raw_ostream &OS, unsigned Indentation = 0,
             unsigned Offset = 0) const;
@@ -109,9 +102,9 @@ class Record final {
 
 private:
   /// Constructor used by Program to create record descriptors.
-  Record(const RecordDecl *, BaseList &&Bases, FieldList &&Fields,
-         VirtualBaseList &&VirtualBases, unsigned VirtualSize,
-         unsigned BaseSize);
+  Record(const RecordDecl *, const Base *Bases, unsigned NumBases,
+         const Field *Fields, unsigned NumFields, Base *VBases,
+         unsigned NumVBases, unsigned VirtualSize, unsigned BaseSize);
 
 private:
   friend class Program;
@@ -119,18 +112,21 @@ class Record final {
   /// Original declaration.
   const RecordDecl *Decl;
   /// List of all base classes.
-  BaseList Bases;
+  const Base *Bases;
+  unsigned NumBases;
   /// List of all the fields in the record.
-  FieldList Fields;
+  const Field *Fields;
+  unsigned NumFields;
   /// List o fall virtual bases.
-  VirtualBaseList VirtualBases;
+  Base *VBases;
+  unsigned NumVBases;
 
   /// Mapping from declarations to bases.
   llvm::DenseMap<const RecordDecl *, const Base *> BaseMap;
   /// Mapping from field identifiers to descriptors.
   llvm::DenseMap<const FieldDecl *, const Field *> FieldMap;
   /// Mapping from declarations to virtual bases.
-  llvm::DenseMap<const RecordDecl *, Base *> VirtualBaseMap;
+  llvm::DenseMap<const RecordDecl *, const Base *> VirtualBaseMap;
   /// Size of the structure.
   unsigned BaseSize;
   /// Size of all virtual bases.

``````````

</details>


https://github.com/llvm/llvm-project/pull/147909
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to