https://github.com/andykaylor created https://github.com/llvm/llvm-project/pull/218545
The LLVM ABI library's RecordType was omitting direct virtual base classes from its base class vector, which was a divergence from the representation in the Clang AST. Clang's AST includes direct virtual bases in both the collection of base classes and the collection of virtual bases. Aligning the handling between the Clang AST and the LLVM ABI RecordType will simplifying porting of ABI classification for future targets. Assisted-by: Cursor / various models >From 93589a71357e3c561ee96b1cd7197dfbbde446e6 Mon Sep 17 00:00:00 2001 From: Andy Kaylor <[email protected]> Date: Mon, 24 Aug 2026 13:50:52 -0700 Subject: [PATCH] [LLVMABI][NFC] Align base class handling with Clang's AST The LLVM ABI library's RecordType was omitting direct virtual base classes from its base class vector, which was a divergence from the representation in the Clang AST. Clang's AST includes direct virtual bases in both the collection of base classes and the collection of virtual bases. Aligning the handling between the Clang AST and the LLVM ABI RecordType will simplifying porting of ABI classification for future targets. Assisted-by: Cursor / various models --- clang/lib/CodeGen/QualTypeMapper.cpp | 21 ++++++++++++++------- llvm/include/llvm/ABI/Types.h | 14 ++++++++++++-- llvm/lib/ABI/Targets/X86.cpp | 6 ++++++ llvm/lib/ABI/Types.cpp | 4 ++++ 4 files changed, 36 insertions(+), 9 deletions(-) diff --git a/clang/lib/CodeGen/QualTypeMapper.cpp b/clang/lib/CodeGen/QualTypeMapper.cpp index 212a138f9b7b7..c8be9de4769c8 100644 --- a/clang/lib/CodeGen/QualTypeMapper.cpp +++ b/clang/lib/CodeGen/QualTypeMapper.cpp @@ -397,16 +397,19 @@ QualTypeMapper::convertCXXRecordType(const CXXRecordDecl *RD) { } for (const auto &Base : RD->bases()) { - if (Base.isVirtual()) - continue; - const RecordType *BaseRT = Base.getType()->castAs<RecordType>(); + const CXXRecordDecl *BaseDecl = BaseRT->getAsCXXRecordDecl(); const llvm::abi::Type *BaseType = convertType(Base.getType()); + // Virtual and non-virtual base offsets live in separate maps in the AST + // record layout. uint64_t BaseOffset = - Layout.getBaseClassOffset(BaseRT->getAsCXXRecordDecl()).getQuantity() * + (Base.isVirtual() ? Layout.getVBaseClassOffset(BaseDecl) + : Layout.getBaseClassOffset(BaseDecl)) + .getQuantity() * 8; - - BaseClasses.emplace_back(BaseType, BaseOffset); + BaseClasses.emplace_back(BaseType, BaseOffset, /*IsBitField=*/false, + /*BitFieldWidth=*/0, /*IsUnnamedBitField=*/false, + /*IsVirtualBase=*/Base.isVirtual()); } for (const auto &VBase : RD->vbases()) { @@ -417,7 +420,11 @@ QualTypeMapper::convertCXXRecordType(const CXXRecordDecl *RD) { .getQuantity() * 8; - VirtualBaseClasses.emplace_back(VBaseType, VBaseOffset); + VirtualBaseClasses.emplace_back(VBaseType, VBaseOffset, + /*IsBitField=*/false, + /*BitFieldWidth=*/0, + /*IsUnnamedBitField=*/false, + /*IsVirtualBase=*/true); } computeFieldInfo(RD, Fields, Layout); diff --git a/llvm/include/llvm/ABI/Types.h b/llvm/include/llvm/ABI/Types.h index 9ae5e8ea49c37..07c0480835794 100644 --- a/llvm/include/llvm/ABI/Types.h +++ b/llvm/include/llvm/ABI/Types.h @@ -237,13 +237,14 @@ struct FieldInfo { uint64_t BitFieldWidth; bool IsBitField; bool IsUnnamedBitfield; + bool IsVirtualBase; FieldInfo(const Type *FieldType, uint64_t OffsetInBits = 0, bool IsBitField = false, uint64_t BitFieldWidth = 0, - bool IsUnnamedBitField = false) + bool IsUnnamedBitField = false, bool IsVirtualBase = false) : FieldType(FieldType), OffsetInBits(OffsetInBits), BitFieldWidth(BitFieldWidth), IsBitField(IsBitField), - IsUnnamedBitfield(IsUnnamedBitField) {} + IsUnnamedBitfield(IsUnnamedBitField), IsVirtualBase(IsVirtualBase) {} LLVM_ABI bool isEmpty() const; }; @@ -304,7 +305,16 @@ class RecordType : public Type { return static_cast<unsigned>(Flags & RecordFlags::IsTransparent) != 0; } ArrayRef<FieldInfo> getFields() const { return Fields; } + + /// Returns the direct base classes, both virtual and non-virtual, mirroring + /// clang::CXXRecordDecl::bases(). A virtual base is marked with + /// FieldInfo::IsVirtualBase, and its offset is only meaningful when this + /// record is the most-derived object. ArrayRef<FieldInfo> getBaseClasses() const { return BaseClasses; } + + /// Returns the virtual base classes, both direct and indirect, mirroring + /// clang::CXXRecordDecl::vbases(). Direct virtual bases therefore appear + /// both here and in getBaseClasses(). ArrayRef<FieldInfo> getVirtualBaseClasses() const { return VirtualBaseClasses; } diff --git a/llvm/lib/ABI/Targets/X86.cpp b/llvm/lib/ABI/Targets/X86.cpp index 43e30c5a43a18..db547b577df32 100644 --- a/llvm/lib/ABI/Targets/X86.cpp +++ b/llvm/lib/ABI/Targets/X86.cpp @@ -512,6 +512,9 @@ void X86_64TargetInfo::classify(const Type *T, uint64_t OffsetBase, Class &Lo, // If this is a C++ record, classify the bases first. if (RT->isCXXRecord()) { for (const auto &Base : RT->getBaseClasses()) { + // A class with a virtual base has a non-trivial copy constructor, so + // getRecordArgABI() above returned before we got here. + assert(!Base.IsVirtualBase && "Unexpected base class!"); // Classify this field. // @@ -942,6 +945,9 @@ static bool bitsContainNoUserData(const Type *Ty, unsigned StartBit, if (RT->isCXXRecord()) { for (unsigned I = 0; I < RT->getNumBaseClasses(); ++I) { const FieldInfo &Base = RT->getBaseClasses()[I]; + // This only runs for types being passed in registers, which cannot + // have virtual bases. + assert(!Base.IsVirtualBase && "Unexpected base class!"); if (Base.OffsetInBits >= EndBit) continue; diff --git a/llvm/lib/ABI/Types.cpp b/llvm/lib/ABI/Types.cpp index 78132aa71fa97..6c44e1f4d1e48 100644 --- a/llvm/lib/ABI/Types.cpp +++ b/llvm/lib/ABI/Types.cpp @@ -39,6 +39,10 @@ RecordType::getElementContainingOffset(unsigned OffsetInBits) const { }; for (const FieldInfo &Base : getBaseClasses()) { + // Direct virtual bases are revisited by the virtual base loop below, which + // also covers the indirect ones. + if (Base.IsVirtualBase) + continue; const auto *BaseRT = dyn_cast<RecordType>(Base.FieldType); if ((!BaseRT || !BaseRT->isEmpty()) && Contains(Base)) return &Base; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
