Skip to content

[clang][bytecode] Allocate Record fields and bases via Program #147909

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tbaederr
Copy link
Contributor

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&to=e0fede973116c4d43e9883586c737c3d0bb99c3e&stat=instructions:u

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

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:bytecode Issues for the clang bytecode constexpr interpreter labels Jul 10, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 10, 2025

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

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&to=e0fede973116c4d43e9883586c737c3d0bb99c3e&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 --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.

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&to=e0fede973116c4d43e9883586c737c3d0bb99c3e&stat=instructions:u

Unfortunately, we still have the three DenseMaps in Record, so they
still heap-allocate memory on their own.
@tbaederr tbaederr changed the title [clang][bytecode] Allocate Record Fields and bases via Program [clang][bytecode] Allocate Record fields and bases via Program Jul 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:bytecode Issues for the clang bytecode constexpr interpreter clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants