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 1 commit
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
60 changes: 41 additions & 19 deletions clang/lib/CIR/CodeGen/CIRGenClass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,31 +238,44 @@ 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());
const mlir::Value oldThisValue = cxxThisValue;

// Initialize virtual bases.
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);
};

for (; b != e && (*b)->isBaseInitializer() && (*b)->isBaseVirtual(); b++) {
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(*b);
}

// The loop above and the loop below could obviously be merged in their
// current form, but when we implement support for the MS C++ ABI, we will
// need to insert a branch after the last virtual base initializer, so
// separate loops will be useful then. The missing code is covered by the
// "virtual base without variants" diagnostic above.

// Handle non-virtual base initializers.
for (; b != e && (*b)->isBaseInitializer(); b++) {
assert(!(*b)->isBaseVirtual());
emitInitializer(*b);
}

cxxThisValue = oldThisValue;
Expand Down Expand Up @@ -370,7 +383,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 +431,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