Skip to content

[CIR] Add Minimal Destructor Definition Support #144719

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 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions clang/include/clang/CIR/MissingFeatures.h
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,10 @@ struct MissingFeatures {
static bool builtinCall() { return false; }
static bool builtinCallF128() { return false; }
static bool builtinCallMathErrno() { return false; }
static bool appleKext() { return false; }
static bool dtorCleanups() { return false; }
static bool completeDtors() { return false; }
static bool vtableInitialization() { return false; }

// Missing types
static bool dataMemberType() { return false; }
Expand Down
7 changes: 7 additions & 0 deletions clang/lib/CIR/CodeGen/CIRGenCXXABI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ void CIRGenCXXABI::buildThisParam(CIRGenFunction &cgf,
assert(!cir::MissingFeatures::cxxabiThisAlignment());
}

cir::GlobalLinkageKind CIRGenCXXABI::getCXXDestructorLinkage(
GVALinkage linkage, const CXXDestructorDecl *dtor, CXXDtorType dt) const {
// Delegate back to cgm by default.
return cgm.getCIRLinkageForDeclarator(dtor, linkage,
/*isConstantVariable=*/false);
}

mlir::Value CIRGenCXXABI::loadIncomingCXXThis(CIRGenFunction &cgf) {
ImplicitParamDecl *vd = getThisDecl(cgf);
Address addr = cgf.getAddrOfLocalVar(vd);
Expand Down
13 changes: 13 additions & 0 deletions clang/lib/CIR/CodeGen/CIRGenCXXABI.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,19 @@ class CIRGenCXXABI {
/// Emit constructor variants required by this ABI.
virtual void emitCXXConstructors(const clang::CXXConstructorDecl *d) = 0;

/// Emit dtor variants required by this ABI.
virtual void emitCXXDestructors(const clang::CXXDestructorDecl *d) = 0;

/// Returns true if the given destructor type should be emitted as a linkonce
/// delegating thunk, regardless of whether the dtor is defined in this TU or
/// not.
virtual bool useThunkForDtorVariant(const CXXDestructorDecl *dtor,
CXXDtorType dt) const = 0;

virtual cir::GlobalLinkageKind
getCXXDestructorLinkage(GVALinkage linkage, const CXXDestructorDecl *dtor,
CXXDtorType dt) const;

/// Returns true if the given constructor or destructor is one of the kinds
/// that the ABI says returns 'this' (only applies when called non-virtually
/// for destructors).
Expand Down
100 changes: 99 additions & 1 deletion clang/lib/CIR/CodeGen/CIRGenFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ cir::FuncOp CIRGenFunction::generateCode(clang::GlobalDecl gd, cir::FuncOp fn,
startFunction(gd, retTy, fn, funcType, args, loc, bodyRange.getBegin());

if (isa<CXXDestructorDecl>(funcDecl))
getCIRGenModule().errorNYI(bodyRange, "C++ destructor definition");
emitDestructorBody(args);
else if (isa<CXXConstructorDecl>(funcDecl))
emitConstructorBody(args);
else if (getLangOpts().CUDA && !getLangOpts().CUDAIsDevice &&
Expand Down Expand Up @@ -538,6 +538,104 @@ void CIRGenFunction::emitConstructorBody(FunctionArgList &args) {
}
}

/// Emits the body of the current destructor.
void CIRGenFunction::emitDestructorBody(FunctionArgList &args) {
const CXXDestructorDecl *dtor = cast<CXXDestructorDecl>(curGD.getDecl());
CXXDtorType dtorType = curGD.getDtorType();

// For an abstract class, non-base destructors are never used (and can't
// be emitted in general, because vbase dtors may not have been validated
// by Sema), but the Itanium ABI doesn't make them optional and Clang may
// in fact emit references to them from other compilations, so emit them
// as functions containing a trap instruction.
if (dtorType != Dtor_Base && dtor->getParent()->isAbstract()) {
SourceLocation loc =
Copy link
Contributor

Choose a reason for hiding this comment

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

It's probably not possible to write a test that hits this code yet. If that's so, can you make it NYI until we can?

dtor->hasBody() ? dtor->getBody()->getBeginLoc() : dtor->getLocation();
builder.create<cir::TrapOp>(getLoc(loc));
// The corresponding clang/CodeGen logic clears the insertion point here,
// but MLIR's builder requires a valid insertion point, so we create a dummy
// block (since the trap is a block terminator).
builder.createBlock(builder.getBlock()->getParent());
return;
}

Stmt *body = dtor->getBody();
if (body)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it's useful to put this assert inside an if.

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

// The call to operator delete in a deleting destructor happens
// outside of the function-try-block, which means it's always
// possible to delegate the destructor body to the complete
// destructor. Do so.
if (dtorType == Dtor_Deleting) {
cgm.errorNYI(dtor->getSourceRange(), "deleting destructor");
return;
}

// If the body is a function-try-block, enter the try before
// anything else.
const bool isTryBody = isa_and_nonnull<CXXTryStmt>(body);
if (isTryBody) {
Copy link
Member

Choose a reason for hiding this comment

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

curly braces not needed here

cgm.errorNYI(dtor->getSourceRange(), "function-try-block destructor");
}

assert(!cir::MissingFeatures::sanitizers());
assert(!cir::MissingFeatures::dtorCleanups());

// If this is the complete variant, just invoke the base variant;
// the epilogue will destruct the virtual bases. But we can't do
// this optimization if the body is a function-try-block, because
// we'd introduce *two* handler blocks. In the Microsoft ABI, we
// always delegate because we might not have a definition in this TU.
switch (dtorType) {
case Dtor_Comdat:
llvm_unreachable("not expecting a COMDAT");
case Dtor_Deleting:
llvm_unreachable("already handled deleting case");

case Dtor_Complete:
assert((body || getTarget().getCXXABI().isMicrosoft()) &&
"can't emit a dtor without a body for non-Microsoft ABIs");

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

// TODO(cir): A complete destructor is supposed to call the base destructor.
// Since we have to emit both dtor kinds we just fall through for now and.
// As long as we don't support virtual bases this should be functionally
// equivalent.
assert(!cir::MissingFeatures::completeDtors());

// Fallthrough: act like we're in the base variant.
[[fallthrough]];

case Dtor_Base:
assert(body);

assert(!cir::MissingFeatures::dtorCleanups());
assert(!cir::MissingFeatures::vtableInitialization());

if (isTryBody) {
cgm.errorNYI(dtor->getSourceRange(), "function-try-block destructor");
} else if (body) {
(void)emitStmt(body, /*useCurrentScope=*/true);
} else {
assert(dtor->isImplicit() && "bodyless dtor not implicit");
// nothing to do besides what's in the epilogue
}
// -fapple-kext must inline any call to this dtor into
// the caller's body.
assert(!cir::MissingFeatures::appleKext());

break;
}

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

// Exit the try if applicable.
if (isTryBody)
cgm.errorNYI(dtor->getSourceRange(), "function-try-block destructor");
}

/// Given a value of type T* that may not be to a complete object, construct
/// an l-vlaue withi the natural pointee alignment of T.
LValue CIRGenFunction::makeNaturalAlignPointeeAddrLValue(mlir::Value val,
Expand Down
1 change: 1 addition & 0 deletions clang/lib/CIR/CodeGen/CIRGenFunction.h
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,7 @@ class CIRGenFunction : public CIRGenTypeCache {
LValue emitCompoundAssignmentLValue(const clang::CompoundAssignOperator *e);

void emitConstructorBody(FunctionArgList &args);
void emitDestructorBody(FunctionArgList &args);

mlir::LogicalResult emitContinueStmt(const clang::ContinueStmt &s);

Expand Down
31 changes: 25 additions & 6 deletions clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,16 @@ class CIRGenItaniumCXXABI : public CIRGenCXXABI {
CIRGenFunction &cgf) override;

void emitCXXConstructors(const clang::CXXConstructorDecl *d) override;
void emitCXXDestructors(const clang::CXXDestructorDecl *d) override;
void emitCXXStructor(clang::GlobalDecl gd) override;

bool useThunkForDtorVariant(const CXXDestructorDecl *dtor,
CXXDtorType dt) const override {
// Itanium does not emit any destructor variant as an inline thunk.
// Delegating may occur as an optimization, but all variants are either
// emitted with external linkage or as linkonce if they are inline and used.
return false;
}
};

} // namespace
Expand Down Expand Up @@ -81,12 +90,6 @@ void CIRGenItaniumCXXABI::emitInstanceFunctionProlog(SourceLocation loc,

void CIRGenItaniumCXXABI::emitCXXStructor(GlobalDecl gd) {
auto *md = cast<CXXMethodDecl>(gd.getDecl());
auto *cd = dyn_cast<CXXConstructorDecl>(md);

if (!cd) {
cgm.errorNYI(md->getSourceRange(), "CXCABI emit destructor");
return;
}

if (cgm.getCodeGenOpts().CXXCtorDtorAliases)
cgm.errorNYI(md->getSourceRange(), "Ctor/Dtor aliases");
Expand All @@ -112,6 +115,22 @@ void CIRGenItaniumCXXABI::emitCXXConstructors(const CXXConstructorDecl *d) {
}
}

void CIRGenItaniumCXXABI::emitCXXDestructors(const CXXDestructorDecl *d) {
// The destructor used for destructing this as a base class; ignores
// virtual bases.
cgm.emitGlobal(GlobalDecl(d, Dtor_Base));

// The destructor used for destructing this as a most-derived class;
// call the base destructor and then destructs any virtual bases.
cgm.emitGlobal(GlobalDecl(d, Dtor_Complete));

// The destructor in a virtual table is always a 'deleting'
// destructor, which calls the complete destructor and then uses the
// appropriate operator delete.
if (d->isVirtual())
cgm.emitGlobal(GlobalDecl(d, Dtor_Deleting));
}

/// Return whether the given global decl needs a VTT (virtual table table)
/// parameter, which it does if it's a base constructor or destructor with
/// virtual bases.
Expand Down
29 changes: 28 additions & 1 deletion clang/lib/CIR/CodeGen/CIRGenModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1024,6 +1024,17 @@ CIRGenModule::getCIRLinkageVarDefinition(const VarDecl *vd, bool isConstant) {
return getCIRLinkageForDeclarator(vd, linkage, isConstant);
}

cir::GlobalLinkageKind CIRGenModule::getFunctionLinkage(GlobalDecl gd) {
const auto *d = cast<FunctionDecl>(gd.getDecl());

GVALinkage linkage = astContext.GetGVALinkageForFunction(d);

if (const auto *dtor = dyn_cast<CXXDestructorDecl>(d))
return getCXXABI().getCXXDestructorLinkage(linkage, dtor, gd.getDtorType());

return getCIRLinkageForDeclarator(d, linkage, /*isConstantVariable=*/false);
}

static cir::GlobalOp
generateStringLiteral(mlir::Location loc, mlir::TypedAttr c,
cir::GlobalLinkageKind lt, CIRGenModule &cgm,
Expand Down Expand Up @@ -1175,6 +1186,9 @@ void CIRGenModule::emitTopLevelDecl(Decl *decl) {
case Decl::CXXConstructor:
getCXXABI().emitCXXConstructors(cast<CXXConstructorDecl>(decl));
break;
case Decl::CXXDestructor:
getCXXABI().emitCXXDestructors(cast<CXXDestructorDecl>(decl));
break;

// C++ Decls
case Decl::LinkageSpec:
Expand Down Expand Up @@ -1236,6 +1250,17 @@ cir::FuncOp CIRGenModule::getAddrOfFunction(clang::GlobalDecl gd,
funcType = convertType(fd->getType());
}

// Devirtualized destructor calls may come through here instead of via
// getAddrOfCXXStructor. Make sure we use the MS ABI base destructor instead
// of the complete destructor when necessary.
if (const auto *dd = dyn_cast<CXXDestructorDecl>(gd.getDecl())) {
if (getTarget().getCXXABI().isMicrosoft() &&
gd.getDtorType() == Dtor_Complete &&
dd->getParent()->getNumVBases() == 0)
errorNYI(dd->getSourceRange(),
"getAddrOfFunction: MS ABI complete destructor");
}

StringRef mangledName = getMangledName(gd);
cir::FuncOp func =
getOrCreateCIRFunction(mangledName, funcType, gd, forVTable, dontDefer,
Expand Down Expand Up @@ -1605,7 +1630,9 @@ cir::FuncOp CIRGenModule::getOrCreateCIRFunction(
// All MSVC dtors other than the base dtor are linkonce_odr and delegate to
// each other bottoming out wiht the base dtor. Therefore we emit non-base
// dtors on usage, even if there is no dtor definition in the TU.
if (isa_and_nonnull<CXXDestructorDecl>(d))
if (isa_and_nonnull<CXXDestructorDecl>(d) &&
getCXXABI().useThunkForDtorVariant(cast<CXXDestructorDecl>(d),
gd.getDtorType()))
errorNYI(d->getSourceRange(), "getOrCreateCIRFunction: dtor");

// This is the first use or definition of a mangled name. If there is a
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/CIR/CodeGen/CIRGenModule.h
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ class CIRGenModule : public CIRGenTypeCache {

cir::GlobalLinkageKind getCIRLinkageVarDefinition(const VarDecl *vd,
bool isConstant);

cir::GlobalLinkageKind getFunctionLinkage(GlobalDecl gd);
/// Helpers to emit "not yet implemented" error diagnostics
DiagnosticBuilder errorNYI(SourceLocation, llvm::StringRef);

Expand Down
3 changes: 2 additions & 1 deletion clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,8 @@ CIRGenTypes::computeRecordLayout(const RecordDecl *rd, cir::RecordType *ty) {
!rd->hasAttr<FinalAttr>()) {
if (lowering.astRecordLayout.getNonVirtualSize() !=
lowering.astRecordLayout.getSize()) {
cgm.errorNYI(rd->getSourceRange(), "computeRecordLayout: CXXRecordDecl");
cgm.errorNYI(rd->getSourceRange(),
"computeRecordLayout: non-POD record layouts");
}
}

Expand Down
50 changes: 50 additions & 0 deletions clang/test/CIR/CodeGen/destructors.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu -Wno-unused-value -fclangir -emit-cir -mno-constructor-aliases %s -o %t.cir
// RUN: FileCheck --input-file=%t.cir %s -check-prefix=CIR
// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu -Wno-unused-value -fclangir -emit-llvm %s -mno-constructor-aliases -o %t-cir.ll
// RUN: FileCheck --input-file=%t-cir.ll %s -check-prefix=LLVM
// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu -Wno-unused-value -emit-llvm %s -mno-constructor-aliases -o %t.ll
// RUN: FileCheck --input-file=%t.ll %s -check-prefix=OGCG

void some_function() noexcept;

struct out_of_line_destructor {
int prevent_tail_padding_reuse;
~out_of_line_destructor();
};

out_of_line_destructor::~out_of_line_destructor() {
some_function();
}

// CIR: !rec_out_of_line_destructor = !cir.record<struct "out_of_line_destructor" {!s32i}>

// CIR: cir.func @_ZN22out_of_line_destructorD2Ev(%{{.+}}: !cir.ptr<!rec_out_of_line_destructor>
// CIR: cir.call @_Z13some_functionv() : () -> ()
// CIR: cir.return

// LLVM: define void @_ZN22out_of_line_destructorD2Ev(ptr %{{.+}})
// LLVM: call void @_Z13some_functionv()
// LLVM: ret void

// OGCG: define dso_local void @_ZN22out_of_line_destructorD2Ev(ptr {{.*}}%{{.+}})
// OGCG: call void @_Z13some_functionv()
// OGCG: ret void

// CIR: cir.func @_ZN22out_of_line_destructorD1Ev(%{{.+}}: !cir.ptr<!rec_out_of_line_destructor>
// CIR: cir.call @_Z13some_functionv() : () -> ()
// CIR: cir.return

// LLVM: define void @_ZN22out_of_line_destructorD1Ev(ptr %{{.+}})
// LLVM: call void @_Z13some_functionv()
// LLVM: ret void

// OGCG: define dso_local void @_ZN22out_of_line_destructorD1Ev(ptr {{.*}}%{{.+}})
// OGCG: call void @_ZN22out_of_line_destructorD2Ev
// OGCG: ret void

struct inline_destructor {
Copy link
Contributor

Choose a reason for hiding this comment

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

We're not emitting code for this case, right? You should probably delete it until we do.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe a CHECK-NOT or couple CHECK-NEXT with a comment?

int prevent_tail_padding_reuse;
~inline_destructor() noexcept(false) {
some_function();
}
};
Loading