Skip to content

[CIR] Add initial support for virtual base classes #155534

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Aug 27, 2025
Merged

[CIR] Add initial support for virtual base classes #155534

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dc7ae0c
[CIR] Add initial support for virtual base classes
andykaylor Jul 11, 2025
5f2fd84
Incorporate review feedback
andykaylor Aug 27, 2025
f1d23b3
Revert unintended clang/codegen changes
andykaylor Aug 27, 2025
758e7e2
Fix formatting
andykaylor Aug 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions clang/lib/CIR/CodeGen/CIRGenCXXABI.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ class CIRGenCXXABI {

void setCXXABIThisValue(CIRGenFunction &cgf, mlir::Value thisPtr);

/// Emit the code to initialize hidden members required to handle virtual
/// inheritance, if needed by the ABI.
virtual void
initializeHiddenVirtualInheritanceMembers(CIRGenFunction &cgf,
const CXXRecordDecl *rd) {}

/// Emit a single constructor/destructor with the gen type from a C++
/// constructor/destructor Decl.
virtual void emitCXXStructor(clang::GlobalDecl gd) = 0;
Expand Down
87 changes: 59 additions & 28 deletions clang/lib/CIR/CodeGen/CIRGenClass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,13 +222,7 @@ void CIRGenFunction::emitCtorPrologue(const CXXConstructorDecl *cd,

const CXXRecordDecl *classDecl = cd->getParent();

// This code doesn't use range-based iteration because we may need to emit
// code between the virtual base initializers and the non-virtual base or
// between the non-virtual base initializers and the member initializers.
CXXConstructorDecl::init_const_iterator b = cd->init_begin(),
e = cd->init_end();

// Virtual base initializers first, if any. They aren't needed if:
// Virtual base initializers aren't needed if:
// - This is a base ctor variant
// - There are no vbases
// - The class is abstract, so a complete object of it cannot be constructed
Expand All @@ -238,31 +232,60 @@ void CIRGenFunction::emitCtorPrologue(const CXXConstructorDecl *cd,
bool constructVBases = ctorType != Ctor_Base &&
classDecl->getNumVBases() != 0 &&
!classDecl->isAbstract();
if (constructVBases) {
cgm.errorNYI(cd->getSourceRange(), "emitCtorPrologue: virtual base");
return;
}

const mlir::Value oldThisValue = cxxThisValue;
if (!constructVBases && b != e && (*b)->isBaseInitializer() &&
(*b)->isBaseVirtual()) {
if (constructVBases &&
!cgm.getTarget().getCXXABI().hasConstructorVariants()) {
cgm.errorNYI(cd->getSourceRange(),
"emitCtorPrologue: virtual base initializer");
"emitCtorPrologue: virtual base without variants");
return;
}

// Handle non-virtual base initializers.
for (; b != e && (*b)->isBaseInitializer(); b++) {
assert(!(*b)->isBaseVirtual());
// Create three separate ranges for the different types of initializers.
auto allInits = cd->inits();

// Find the boundaries between the three groups.
auto virtualBaseEnd = std::find_if(
allInits.begin(), allInits.end(), [](const CXXCtorInitializer *Init) {
return !(Init->isBaseInitializer() && Init->isBaseVirtual());
});

auto nonVirtualBaseEnd = std::find_if(virtualBaseEnd, allInits.end(),
[](const CXXCtorInitializer *Init) {
return !Init->isBaseInitializer();
});

// Create the three ranges.
auto virtualBaseInits = llvm::make_range(allInits.begin(), virtualBaseEnd);
auto nonVirtualBaseInits =
llvm::make_range(virtualBaseEnd, nonVirtualBaseEnd);
auto memberInits = llvm::make_range(nonVirtualBaseEnd, allInits.end());

const mlir::Value oldThisValue = cxxThisValue;

auto emitInitializer = [&](CXXCtorInitializer *baseInit) {
if (cgm.getCodeGenOpts().StrictVTablePointers &&
cgm.getCodeGenOpts().OptimizationLevel > 0 &&
isInitializerOfDynamicClass(*b)) {
isInitializerOfDynamicClass(baseInit)) {
// It's OK to continue after emitting the error here. The missing code
// just "launders" the 'this' pointer.
cgm.errorNYI(cd->getSourceRange(),
"emitCtorPrologue: strict vtable pointers");
return;
"emitCtorPrologue: strict vtable pointers for vbase");
}
emitBaseInitializer(getLoc(cd->getBeginLoc()), classDecl, *b);
emitBaseInitializer(getLoc(cd->getBeginLoc()), classDecl, baseInit);
};

// Process virtual base initializers.
for (CXXCtorInitializer *virtualBaseInit : virtualBaseInits) {
if (!constructVBases)
Copy link
Collaborator

@erichkeane erichkeane Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this inside the loop rather than preventing this loop from happening at all?

Also, is there any sort of range loop we can use here? Thisis awful ugly...

Copy link
Contributor Author

@andykaylor andykaylor Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is inside the loop because if we are not constructing vbases we need to update the iterator to move past them. The initializers are ordered such that all virtual bases occur before the first non-virtual base. This loop is necessary to position the iterator for the loop below.

The MS CXXABI throws a bit of a wrench into the possibility of a range loop, because we need to insert a branch to a continue block between the virtual initializers and the non-virtual initializers, but we can probably use a flag to check for the conditions to do that. I'll play around with this loop in OGCG and see if I can simplify it without breaking anything.

Copy link
Collaborator

@erichkeane erichkeane Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right! I missed the same iterators. Anything you can do to make this less awful would be appreciated :)

Copy link
Contributor Author

@andykaylor andykaylor Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm proceeding with the implementation change modeled after the classic codegen change proposed in #155668. If that ends up being rejected, I'll update this accordingly.

continue;
emitInitializer(virtualBaseInit);
}

assert(!cir::MissingFeatures::msabi());

// Then, non-virtual base initializers.
for (CXXCtorInitializer *nonVirtualBaseInit : nonVirtualBaseInits) {
assert(!nonVirtualBaseInit->isBaseVirtual());
emitInitializer(nonVirtualBaseInit);
}

cxxThisValue = oldThisValue;
Expand All @@ -276,8 +299,7 @@ void CIRGenFunction::emitCtorPrologue(const CXXConstructorDecl *cd,
// lowering or optimization phases to keep the memory accesses more
// explicit. For now, we don't insert memcpy at all.
assert(!cir::MissingFeatures::ctorMemcpyizer());
for (; b != e; b++) {
CXXCtorInitializer *member = (*b);
for (CXXCtorInitializer *member : memberInits) {
assert(!member->isBaseInitializer());
assert(member->isAnyMemberInitializer() &&
"Delegating initializer on non-delegating constructor");
Expand Down Expand Up @@ -370,7 +392,7 @@ void CIRGenFunction::initializeVTablePointers(mlir::Location loc,
initializeVTablePointer(loc, vptr);

if (rd->getNumVBases())
cgm.errorNYI(loc, "initializeVTablePointers: virtual base");
cgm.getCXXABI().initializeHiddenVirtualInheritanceMembers(*this, rd);
}

CIRGenFunction::VPtrsVector
Expand Down Expand Up @@ -418,8 +440,17 @@ void CIRGenFunction::getVTablePointers(BaseSubobject base,
const CXXRecordDecl *nextBaseDecl;

if (nextBase.isVirtual()) {
cgm.errorNYI(rd->getSourceRange(), "getVTablePointers: virtual base");
return;
// Check if we've visited this virtual base before.
if (!vbases.insert(baseDecl).second)
continue;

const ASTRecordLayout &layout =
getContext().getASTRecordLayout(vtableClass);

nextBaseDecl = nearestVBase;
baseOffset = layout.getVBaseClassOffset(baseDecl);
baseOffsetFromNearestVBase = CharUnits::Zero();
baseDeclIsNonVirtualPrimaryBase = false;
} else {
const ASTRecordLayout &layout = getContext().getASTRecordLayout(rd);

Expand Down
8 changes: 2 additions & 6 deletions clang/lib/CIR/CodeGen/CIRGenExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1972,12 +1972,8 @@ void CIRGenFunction::emitCXXConstructExpr(const CXXConstructExpr *e,
delegating = true;
break;
case CXXConstructionKind::VirtualBase:
// This should just set 'forVirtualBase' to true and fall through, but
// virtual base class support is otherwise missing, so this needs to wait
// until it can be tested.
cgm.errorNYI(e->getSourceRange(),
"emitCXXConstructExpr: virtual base constructor");
return;
forVirtualBase = true;
[[fallthrough]];
case CXXConstructionKind::NonVirtualBase:
type = Ctor_Base;
break;
Expand Down
5 changes: 3 additions & 2 deletions clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,9 +487,10 @@ mlir::Value CIRGenItaniumCXXABI::getVTableAddressPointInStructor(
CIRGenFunction &cgf, const clang::CXXRecordDecl *vtableClass,
clang::BaseSubobject base, const clang::CXXRecordDecl *nearestVBase) {

if (base.getBase()->getNumVBases()) {
if ((base.getBase()->getNumVBases() || nearestVBase != nullptr) &&
needsVTTParameter(cgf.curGD)) {
cgm.errorNYI(cgf.curFuncDecl->getLocation(),
"getVTableAddressPointInStructor: virtual base");
"getVTableAddressPointInStructorWithVTT");
}
return getVTableAddressPoint(base, vtableClass);
}
Expand Down
4 changes: 4 additions & 0 deletions clang/lib/CIR/CodeGen/CIRGenRecordLayout.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,10 @@ class CIRGenRecordLayout {
// for both virtual and non-virtual bases.
llvm::DenseMap<const clang::CXXRecordDecl *, unsigned> nonVirtualBases;

/// Map from virtual bases to their field index in the complete object.
llvm::DenseMap<const clang::CXXRecordDecl *, unsigned>
completeObjectVirtualBases;

/// Map from (bit-field) record field to the corresponding CIR record type
/// field no. This info is populated by record builder.
llvm::DenseMap<const clang::FieldDecl *, CIRGenBitFieldInfo> bitFields;
Expand Down
Loading
Loading