llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Morris Hafner (mmha) <details> <summary>Changes</summary> This patch upstreams support for writing inline and out of line C++ destructor definitions. Calling a destructor implcitly or explicitly is left for a future patch. Because of that restriction complete destructors (D2 in Itanium mangling) do not call into the base (D1) destructors yet but simply behave like a base destructor. Deleting (D0) destructor support is not part of this patch. Destructor aliases aren't supported, either. Because of this compilation with -mno-constructor-aliases may be required to avoid running into NYI errors. --- Full diff: https://github.com/llvm/llvm-project/pull/144719.diff 10 Files Affected: - (modified) clang/include/clang/CIR/MissingFeatures.h (+4) - (modified) clang/lib/CIR/CodeGen/CIRGenCXXABI.cpp (+7) - (modified) clang/lib/CIR/CodeGen/CIRGenCXXABI.h (+13) - (modified) clang/lib/CIR/CodeGen/CIRGenFunction.cpp (+99-1) - (modified) clang/lib/CIR/CodeGen/CIRGenFunction.h (+1) - (modified) clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp (+25-6) - (modified) clang/lib/CIR/CodeGen/CIRGenModule.cpp (+28-1) - (modified) clang/lib/CIR/CodeGen/CIRGenModule.h (+1-1) - (modified) clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp (+2-1) - (added) clang/test/CIR/CodeGen/destructors.cpp (+50) ``````````diff diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h index 3d120903dea19..174b1ccfd27f9 100644 --- a/clang/include/clang/CIR/MissingFeatures.h +++ b/clang/include/clang/CIR/MissingFeatures.h @@ -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; } diff --git a/clang/lib/CIR/CodeGen/CIRGenCXXABI.cpp b/clang/lib/CIR/CodeGen/CIRGenCXXABI.cpp index 6cf4e5c658fb6..33b812ac81f6e 100644 --- a/clang/lib/CIR/CodeGen/CIRGenCXXABI.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenCXXABI.cpp @@ -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); diff --git a/clang/lib/CIR/CodeGen/CIRGenCXXABI.h b/clang/lib/CIR/CodeGen/CIRGenCXXABI.h index 2d967fd307e01..eb079b877b7ff 100644 --- a/clang/lib/CIR/CodeGen/CIRGenCXXABI.h +++ b/clang/lib/CIR/CodeGen/CIRGenCXXABI.h @@ -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). diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp index fd413fe86383a..e7f91480cc0f7 100644 --- a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp @@ -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 && @@ -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 = + 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) + 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("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) { + cgm.errorNYI("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("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("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, diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h index 6c490a72b2e93..4fadc4e033b11 100644 --- a/clang/lib/CIR/CodeGen/CIRGenFunction.h +++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h @@ -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); diff --git a/clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp b/clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp index cd9096a0188a7..889515c335002 100644 --- a/clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp @@ -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 @@ -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"); @@ -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. diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp index 434dd376208e1..efa76f9be540b 100644 --- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp @@ -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, @@ -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: @@ -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, @@ -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 diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.h b/clang/lib/CIR/CodeGen/CIRGenModule.h index 0ea2d9f9c8229..b901784cf0915 100644 --- a/clang/lib/CIR/CodeGen/CIRGenModule.h +++ b/clang/lib/CIR/CodeGen/CIRGenModule.h @@ -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); diff --git a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp index 0aeef7fd89aef..4447a6dbe801a 100644 --- a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp @@ -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"); } } diff --git a/clang/test/CIR/CodeGen/destructors.cpp b/clang/test/CIR/CodeGen/destructors.cpp new file mode 100644 index 0000000000000..658a537b4cd46 --- /dev/null +++ b/clang/test/CIR/CodeGen/destructors.cpp @@ -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 { + int prevent_tail_padding_reuse; + ~inline_destructor() noexcept(false) { + some_function(); + } +}; `````````` </details> https://github.com/llvm/llvm-project/pull/144719 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits