https://github.com/davidstone created https://github.com/llvm/llvm-project/pull/168768
This simplifies the code by removing the manual optimization for size == 1, and also gives us an optimization for other small sizes. Accept a `llvm::SmallVector` by value for the constructor and move it into the destination, rather than accepting `ArrayRef` that we copy from. This also lets us not have to construct a reference to the elements of a `std::initializer_list`, which requires reading the implementation of the constructor to know whether it's safe. Also explicitly document that the constructor requires the input indexes to have a size of at least 1. >From 8eae3a43847a4667d4660b30af4f6da980462ea6 Mon Sep 17 00:00:00 2001 From: David Stone <[email protected]> Date: Wed, 19 Nov 2025 12:58:07 -0700 Subject: [PATCH] Use `llvm::SmallVector` instead of `OwningArrayRef` in `VTableLayout`. This simplifies the code by removing the manual optimization for size == 1, and also gives us an optimization for other small sizes. Accept a `llvm::SmallVector` by value for the constructor and move it into the destination, rather than accepting `ArrayRef` that we copy from. This also lets us not have to construct a reference to the elements of a `std::initializer_list`, which requires reading the implementation of the constructor to know whether it's safe. Also explicitly document that the constructor requires the input indexes to have a size of at least 1. --- clang/include/clang/AST/VTableBuilder.h | 32 +++++++------------------ clang/lib/AST/VTableBuilder.cpp | 17 ++++++------- 2 files changed, 16 insertions(+), 33 deletions(-) diff --git a/clang/include/clang/AST/VTableBuilder.h b/clang/include/clang/AST/VTableBuilder.h index e1efe8cddcc5e..58f6fcbc1270e 100644 --- a/clang/include/clang/AST/VTableBuilder.h +++ b/clang/include/clang/AST/VTableBuilder.h @@ -246,12 +246,12 @@ class VTableLayout { // point for a given vtable index. typedef llvm::SmallVector<unsigned, 4> AddressPointsIndexMapTy; + using VTableIndicesTy = llvm::SmallVector<std::size_t>; + private: - // Stores the component indices of the first component of each virtual table in - // the virtual table group. To save a little memory in the common case where - // the vtable group contains a single vtable, an empty vector here represents - // the vector {0}. - OwningArrayRef<size_t> VTableIndices; + // Stores the component indices of the first component of each virtual table + // in the virtual table group. + VTableIndicesTy VTableIndices; OwningArrayRef<VTableComponent> VTableComponents; @@ -265,7 +265,8 @@ class VTableLayout { AddressPointsIndexMapTy AddressPointIndices; public: - VTableLayout(ArrayRef<size_t> VTableIndices, + // Requires `!VTableIndicies.empty()` + VTableLayout(VTableIndicesTy VTableIndices, ArrayRef<VTableComponent> VTableComponents, ArrayRef<VTableThunkTy> VTableThunks, const AddressPointsMapTy &AddressPoints); @@ -292,26 +293,11 @@ class VTableLayout { return AddressPointIndices; } - size_t getNumVTables() const { - if (VTableIndices.empty()) - return 1; - return VTableIndices.size(); - } + size_t getNumVTables() const { return VTableIndices.size(); } - size_t getVTableOffset(size_t i) const { - if (VTableIndices.empty()) { - assert(i == 0); - return 0; - } - return VTableIndices[i]; - } + size_t getVTableOffset(size_t i) const { return VTableIndices[i]; } size_t getVTableSize(size_t i) const { - if (VTableIndices.empty()) { - assert(i == 0); - return vtable_components().size(); - } - size_t thisIndex = VTableIndices[i]; size_t nextIndex = (i + 1 == VTableIndices.size()) ? vtable_components().size() diff --git a/clang/lib/AST/VTableBuilder.cpp b/clang/lib/AST/VTableBuilder.cpp index 9951126c2c3a3..be20aebfc382b 100644 --- a/clang/lib/AST/VTableBuilder.cpp +++ b/clang/lib/AST/VTableBuilder.cpp @@ -999,7 +999,7 @@ class ItaniumVTableBuilder { public: /// Component indices of the first component of each of the vtables in the /// vtable group. - SmallVector<size_t, 4> VTableIndices; + VTableLayout::VTableIndicesTy VTableIndices; ItaniumVTableBuilder(ItaniumVTableContext &VTables, const CXXRecordDecl *MostDerivedClass, @@ -2306,18 +2306,15 @@ MakeAddressPointIndices(const VTableLayout::AddressPointsMapTy &addressPoints, return indexMap; } -VTableLayout::VTableLayout(ArrayRef<size_t> VTableIndices, +VTableLayout::VTableLayout(VTableIndicesTy VTableIndices, ArrayRef<VTableComponent> VTableComponents, ArrayRef<VTableThunkTy> VTableThunks, const AddressPointsMapTy &AddressPoints) - : VTableComponents(VTableComponents), VTableThunks(VTableThunks), + : VTableIndices(std::move(VTableIndices)), + VTableComponents(VTableComponents), VTableThunks(VTableThunks), AddressPoints(AddressPoints), AddressPointIndices(MakeAddressPointIndices( AddressPoints, VTableIndices.size())) { - if (VTableIndices.size() <= 1) - assert(VTableIndices.size() == 1 && VTableIndices[0] == 0); - else - this->VTableIndices = OwningArrayRef<size_t>(VTableIndices); - + assert(!VTableIndices.empty() && "VTableLayout requires at least one index."); llvm::sort(this->VTableThunks, [](const VTableLayout::VTableThunkTy &LHS, const VTableLayout::VTableThunkTy &RHS) { assert((LHS.first != RHS.first || LHS.second == RHS.second) && @@ -3730,8 +3727,8 @@ void MicrosoftVTableContext::computeVTableRelatedInformation( SmallVector<VTableLayout::VTableThunkTy, 1> VTableThunks( Builder.vtable_thunks_begin(), Builder.vtable_thunks_end()); VFTableLayouts[id] = std::make_unique<VTableLayout>( - ArrayRef<size_t>{0}, Builder.vtable_components(), VTableThunks, - EmptyAddressPointsMap); + VTableLayout::VTableIndicesTy{0}, Builder.vtable_components(), + VTableThunks, EmptyAddressPointsMap); Thunks.insert(Builder.thunks_begin(), Builder.thunks_end()); const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
