Author: hliao Date: Fri Oct 18 17:15:19 2019 New Revision: 375309 URL: http://llvm.org/viewvc/llvm-project?rev=375309&view=rev Log: [hip][cuda] Fix the extended lambda name mangling issue.
Summary: - HIP/CUDA host side needs to use device kernel symbol name to match the device side binaries. Without a consistent naming between host- and device-side compilations, it's risky that wrong device binaries are executed. Consistent naming is usually not an issue until unnamed types are used, especially the lambda. In this patch, the consistent name mangling is addressed for the extended lambdas, i.e. the lambdas annotated with `__device__`. - In [Itanium C++ ABI][1], the mangling of the lambda is generally unspecified unless, in certain cases, ODR rule is required to ensure consisent naming cross TUs. The extended lambda is such a case as its name may be part of a device kernel function, e.g., the extended lambda is used as a template argument and etc. Thus, we need to force ODR for extended lambdas as they are referenced in both device- and host-side TUs. Furthermore, if a extended lambda is nested in other (extended or not) lambdas, those lambdas are required to follow ODR naming as well. This patch revises the current lambda mangle numbering to force ODR from an extended lambda to all its parent lambdas. - On the other side, the aforementioned ODR naming should not change those lambdas' original linkages, i.e., we cannot replace the original `internal` with `linkonce_odr`; otherwise, we may violate ODR in general. This patch introduces a new field `HasKnownInternalLinkage` in lambda data to decouple the current linkage calculation based on mangling number assigned. [1]: https://itanium-cxx-abi.github.io/cxx-abi/abi.html Reviewers: tra, rsmith, yaxunl, martong, shafik Subscribers: cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D68818 Added: cfe/trunk/test/CodeGenCUDA/unnamed-types.cu Modified: cfe/trunk/include/clang/AST/DeclCXX.h cfe/trunk/include/clang/Sema/Sema.h cfe/trunk/lib/AST/ASTImporter.cpp cfe/trunk/lib/AST/Decl.cpp cfe/trunk/lib/Sema/SemaLambda.cpp cfe/trunk/lib/Sema/TreeTransform.h cfe/trunk/lib/Serialization/ASTReaderDecl.cpp cfe/trunk/lib/Serialization/ASTWriter.cpp Modified: cfe/trunk/include/clang/AST/DeclCXX.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=375309&r1=375308&r2=375309&view=diff ============================================================================== --- cfe/trunk/include/clang/AST/DeclCXX.h (original) +++ cfe/trunk/include/clang/AST/DeclCXX.h Fri Oct 18 17:15:19 2019 @@ -389,9 +389,12 @@ class CXXRecordDecl : public RecordDecl /// The number of explicit captures in this lambda. unsigned NumExplicitCaptures : 13; + /// Has known `internal` linkage. + unsigned HasKnownInternalLinkage : 1; + /// The number used to indicate this lambda expression for name /// mangling in the Itanium C++ ABI. - unsigned ManglingNumber = 0; + unsigned ManglingNumber : 31; /// The declaration that provides context for this lambda, if the /// actual DeclContext does not suffice. This is used for lambdas that @@ -406,12 +409,12 @@ class CXXRecordDecl : public RecordDecl /// The type of the call method. TypeSourceInfo *MethodTyInfo; - LambdaDefinitionData(CXXRecordDecl *D, TypeSourceInfo *Info, - bool Dependent, bool IsGeneric, - LambdaCaptureDefault CaptureDefault) - : DefinitionData(D), Dependent(Dependent), IsGenericLambda(IsGeneric), - CaptureDefault(CaptureDefault), NumCaptures(0), NumExplicitCaptures(0), - MethodTyInfo(Info) { + LambdaDefinitionData(CXXRecordDecl *D, TypeSourceInfo *Info, bool Dependent, + bool IsGeneric, LambdaCaptureDefault CaptureDefault) + : DefinitionData(D), Dependent(Dependent), IsGenericLambda(IsGeneric), + CaptureDefault(CaptureDefault), NumCaptures(0), + NumExplicitCaptures(0), HasKnownInternalLinkage(0), ManglingNumber(0), + MethodTyInfo(Info) { IsLambda = true; // C++1z [expr.prim.lambda]p4: @@ -1705,6 +1708,13 @@ public: return getLambdaData().ManglingNumber; } + /// The lambda is known to has internal linkage no matter whether it has name + /// mangling number. + bool hasKnownLambdaInternalLinkage() const { + assert(isLambda() && "Not a lambda closure type!"); + return getLambdaData().HasKnownInternalLinkage; + } + /// Retrieve the declaration that provides additional context for a /// lambda, when the normal declaration context is not specific enough. /// @@ -1718,9 +1728,12 @@ public: /// Set the mangling number and context declaration for a lambda /// class. - void setLambdaMangling(unsigned ManglingNumber, Decl *ContextDecl) { + void setLambdaMangling(unsigned ManglingNumber, Decl *ContextDecl, + bool HasKnownInternalLinkage = false) { + assert(isLambda() && "Not a lambda closure type!"); getLambdaData().ManglingNumber = ManglingNumber; getLambdaData().ContextDecl = ContextDecl; + getLambdaData().HasKnownInternalLinkage = HasKnownInternalLinkage; } /// Returns the inheritance model used for this record. Modified: cfe/trunk/include/clang/Sema/Sema.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=375309&r1=375308&r2=375309&view=diff ============================================================================== --- cfe/trunk/include/clang/Sema/Sema.h (original) +++ cfe/trunk/include/clang/Sema/Sema.h Fri Oct 18 17:15:19 2019 @@ -5926,12 +5926,17 @@ public: LambdaCaptureDefault CaptureDefault); /// Start the definition of a lambda expression. - CXXMethodDecl * - startLambdaDefinition(CXXRecordDecl *Class, SourceRange IntroducerRange, - TypeSourceInfo *MethodType, SourceLocation EndLoc, - ArrayRef<ParmVarDecl *> Params, - ConstexprSpecKind ConstexprKind, - Optional<std::pair<unsigned, Decl *>> Mangling = None); + CXXMethodDecl *startLambdaDefinition(CXXRecordDecl *Class, + SourceRange IntroducerRange, + TypeSourceInfo *MethodType, + SourceLocation EndLoc, + ArrayRef<ParmVarDecl *> Params, + ConstexprSpecKind ConstexprKind); + + /// Number lambda for linkage purposes if necessary. + void handleLambdaNumbering( + CXXRecordDecl *Class, CXXMethodDecl *Method, + Optional<std::tuple<unsigned, bool, Decl *>> Mangling = None); /// Endow the lambda scope info with the relevant properties. void buildLambdaScope(sema::LambdaScopeInfo *LSI, Modified: cfe/trunk/lib/AST/ASTImporter.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=375309&r1=375308&r2=375309&view=diff ============================================================================== --- cfe/trunk/lib/AST/ASTImporter.cpp (original) +++ cfe/trunk/lib/AST/ASTImporter.cpp Fri Oct 18 17:15:19 2019 @@ -2694,7 +2694,8 @@ ExpectedDecl ASTNodeImporter::VisitRecor ExpectedDecl CDeclOrErr = import(DCXX->getLambdaContextDecl()); if (!CDeclOrErr) return CDeclOrErr.takeError(); - D2CXX->setLambdaMangling(DCXX->getLambdaManglingNumber(), *CDeclOrErr); + D2CXX->setLambdaMangling(DCXX->getLambdaManglingNumber(), *CDeclOrErr, + DCXX->hasKnownLambdaInternalLinkage()); } else if (DCXX->isInjectedClassName()) { // We have to be careful to do a similar dance to the one in // Sema::ActOnStartCXXMemberDeclarations Modified: cfe/trunk/lib/AST/Decl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=375309&r1=375308&r2=375309&view=diff ============================================================================== --- cfe/trunk/lib/AST/Decl.cpp (original) +++ cfe/trunk/lib/AST/Decl.cpp Fri Oct 18 17:15:19 2019 @@ -1385,7 +1385,8 @@ LinkageInfo LinkageComputer::computeLVFo case Decl::CXXRecord: { const auto *Record = cast<CXXRecordDecl>(D); if (Record->isLambda()) { - if (!Record->getLambdaManglingNumber()) { + if (Record->hasKnownLambdaInternalLinkage() || + !Record->getLambdaManglingNumber()) { // This lambda has no mangling number, so it's internal. return getInternalLinkageFor(D); } @@ -1402,7 +1403,8 @@ LinkageInfo LinkageComputer::computeLVFo // }; const CXXRecordDecl *OuterMostLambda = getOutermostEnclosingLambda(Record); - if (!OuterMostLambda->getLambdaManglingNumber()) + if (OuterMostLambda->hasKnownLambdaInternalLinkage() || + !OuterMostLambda->getLambdaManglingNumber()) return getInternalLinkageFor(D); return getLVForClosure( Modified: cfe/trunk/lib/Sema/SemaLambda.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLambda.cpp?rev=375309&r1=375308&r2=375309&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaLambda.cpp (original) +++ cfe/trunk/lib/Sema/SemaLambda.cpp Fri Oct 18 17:15:19 2019 @@ -335,7 +335,7 @@ Sema::getCurrentMangleNumberContext(cons case StaticDataMember: // -- the initializers of nonspecialized static members of template classes if (!IsInNonspecializedTemplate) - return std::make_tuple(nullptr, nullptr); + return std::make_tuple(nullptr, ManglingContextDecl); // Fall through to get the current context. LLVM_FALLTHROUGH; @@ -356,14 +356,15 @@ Sema::getCurrentMangleNumberContext(cons llvm_unreachable("unexpected context"); } -CXXMethodDecl *Sema::startLambdaDefinition( - CXXRecordDecl *Class, SourceRange IntroducerRange, - TypeSourceInfo *MethodTypeInfo, SourceLocation EndLoc, - ArrayRef<ParmVarDecl *> Params, ConstexprSpecKind ConstexprKind, - Optional<std::pair<unsigned, Decl *>> Mangling) { +CXXMethodDecl *Sema::startLambdaDefinition(CXXRecordDecl *Class, + SourceRange IntroducerRange, + TypeSourceInfo *MethodTypeInfo, + SourceLocation EndLoc, + ArrayRef<ParmVarDecl *> Params, + ConstexprSpecKind ConstexprKind) { QualType MethodType = MethodTypeInfo->getType(); TemplateParameterList *TemplateParams = - getGenericLambdaTemplateParameterList(getCurLambda(), *this); + getGenericLambdaTemplateParameterList(getCurLambda(), *this); // If a lambda appears in a dependent context or is a generic lambda (has // template parameters) and has an 'auto' return type, deduce it to a // dependent type. @@ -425,20 +426,55 @@ CXXMethodDecl *Sema::startLambdaDefiniti P->setOwningFunction(Method); } + return Method; +} + +void Sema::handleLambdaNumbering( + CXXRecordDecl *Class, CXXMethodDecl *Method, + Optional<std::tuple<unsigned, bool, Decl *>> Mangling) { if (Mangling) { - Class->setLambdaMangling(Mangling->first, Mangling->second); - } else { - MangleNumberingContext *MCtx; + unsigned ManglingNumber; + bool HasKnownInternalLinkage; Decl *ManglingContextDecl; - std::tie(MCtx, ManglingContextDecl) = - getCurrentMangleNumberContext(Class->getDeclContext()); - if (MCtx) { - unsigned ManglingNumber = MCtx->getManglingNumber(Method); - Class->setLambdaMangling(ManglingNumber, ManglingContextDecl); - } + std::tie(ManglingNumber, HasKnownInternalLinkage, ManglingContextDecl) = + Mangling.getValue(); + Class->setLambdaMangling(ManglingNumber, ManglingContextDecl, + HasKnownInternalLinkage); + return; } - return Method; + auto getMangleNumberingContext = + [this](CXXRecordDecl *Class, Decl *ManglingContextDecl) -> MangleNumberingContext * { + // Get mangle numbering context if there's any extra decl context. + if (ManglingContextDecl) + return &Context.getManglingNumberContext( + ASTContext::NeedExtraManglingDecl, ManglingContextDecl); + // Otherwise, from that lambda's decl context. + auto DC = Class->getDeclContext(); + while (auto *CD = dyn_cast<CapturedDecl>(DC)) + DC = CD->getParent(); + return &Context.getManglingNumberContext(DC); + }; + + MangleNumberingContext *MCtx; + Decl *ManglingContextDecl; + std::tie(MCtx, ManglingContextDecl) = + getCurrentMangleNumberContext(Class->getDeclContext()); + bool HasKnownInternalLinkage = false; + if (!MCtx && getLangOpts().CUDA) { + // Force lambda numbering in CUDA/HIP as we need to name lambdas following + // ODR. Both device- and host-compilation need to have a consistent naming + // on kernel functions. As lambdas are potential part of these `__global__` + // function names, they needs numbering following ODR. + MCtx = getMangleNumberingContext(Class, ManglingContextDecl); + assert(MCtx && "Retrieving mangle numbering context failed!"); + HasKnownInternalLinkage = true; + } + if (MCtx) { + unsigned ManglingNumber = MCtx->getManglingNumber(Method); + Class->setLambdaMangling(ManglingNumber, ManglingContextDecl, + HasKnownInternalLinkage); + } } void Sema::buildLambdaScope(LambdaScopeInfo *LSI, @@ -951,6 +987,9 @@ void Sema::ActOnStartOfLambdaDefinition( if (getLangOpts().CUDA) CUDASetLambdaAttrs(Method); + // Number the lambda for linkage purposes if necessary. + handleLambdaNumbering(Class, Method); + // Introduce the function call operator as the current declaration context. PushDeclContext(CurScope, Method); Modified: cfe/trunk/lib/Sema/TreeTransform.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=375309&r1=375308&r2=375309&view=diff ============================================================================== --- cfe/trunk/lib/Sema/TreeTransform.h (original) +++ cfe/trunk/lib/Sema/TreeTransform.h Fri Oct 18 17:15:19 2019 @@ -11497,17 +11497,18 @@ TreeTransform<Derived>::TransformLambdaE E->getCaptureDefault()); getDerived().transformedLocalDecl(OldClass, {Class}); - Optional<std::pair<unsigned, Decl*>> Mangling; + Optional<std::tuple<unsigned, bool, Decl *>> Mangling; if (getDerived().ReplacingOriginal()) - Mangling = std::make_pair(OldClass->getLambdaManglingNumber(), - OldClass->getLambdaContextDecl()); + Mangling = std::make_tuple(OldClass->getLambdaManglingNumber(), + OldClass->hasKnownLambdaInternalLinkage(), + OldClass->getLambdaContextDecl()); // Build the call operator. CXXMethodDecl *NewCallOperator = getSema().startLambdaDefinition( Class, E->getIntroducerRange(), NewCallOpTSI, E->getCallOperator()->getEndLoc(), NewCallOpTSI->getTypeLoc().castAs<FunctionProtoTypeLoc>().getParams(), - E->getCallOperator()->getConstexprKind(), Mangling); + E->getCallOperator()->getConstexprKind()); LSI->CallOperator = NewCallOperator; @@ -11527,6 +11528,9 @@ TreeTransform<Derived>::TransformLambdaE getDerived().transformAttrs(E->getCallOperator(), NewCallOperator); getDerived().transformedLocalDecl(E->getCallOperator(), {NewCallOperator}); + // Number the lambda for linkage purposes if necessary. + getSema().handleLambdaNumbering(Class, NewCallOperator, Mangling); + // Introduce the context of the call operator. Sema::ContextRAII SavedContext(getSema(), NewCallOperator, /*NewThisContext*/false); Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=375309&r1=375308&r2=375309&view=diff ============================================================================== --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original) +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Fri Oct 18 17:15:19 2019 @@ -1690,6 +1690,7 @@ void ASTDeclReader::ReadCXXDefinitionDat Lambda.CaptureDefault = Record.readInt(); Lambda.NumCaptures = Record.readInt(); Lambda.NumExplicitCaptures = Record.readInt(); + Lambda.HasKnownInternalLinkage = Record.readInt(); Lambda.ManglingNumber = Record.readInt(); Lambda.ContextDecl = ReadDeclID(); Lambda.Captures = (Capture *)Reader.getContext().Allocate( Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=375309&r1=375308&r2=375309&view=diff ============================================================================== --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original) +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Fri Oct 18 17:15:19 2019 @@ -6224,6 +6224,7 @@ void ASTRecordWriter::AddCXXDefinitionDa Record->push_back(Lambda.CaptureDefault); Record->push_back(Lambda.NumCaptures); Record->push_back(Lambda.NumExplicitCaptures); + Record->push_back(Lambda.HasKnownInternalLinkage); Record->push_back(Lambda.ManglingNumber); AddDeclRef(D->getLambdaContextDecl()); AddTypeSourceInfo(Lambda.MethodTyInfo); Added: cfe/trunk/test/CodeGenCUDA/unnamed-types.cu URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCUDA/unnamed-types.cu?rev=375309&view=auto ============================================================================== --- cfe/trunk/test/CodeGenCUDA/unnamed-types.cu (added) +++ cfe/trunk/test/CodeGenCUDA/unnamed-types.cu Fri Oct 18 17:15:19 2019 @@ -0,0 +1,39 @@ +// RUN: %clang_cc1 -std=c++11 -x hip -triple x86_64-linux-gnu -aux-triple amdgcn-amd-amdhsa -emit-llvm %s -o - | FileCheck %s --check-prefix=HOST +// RUN: %clang_cc1 -std=c++11 -x hip -triple amdgcn-amd-amdhsa -fcuda-is-device -emit-llvm %s -o - | FileCheck %s --check-prefix=DEVICE + +#include "Inputs/cuda.h" + +// HOST: @0 = private unnamed_addr constant [43 x i8] c"_Z2k0IZZ2f1PfENKUlS0_E_clES0_EUlfE_EvS0_T_\00", align 1 + +__device__ float d0(float x) { + return [](float x) { return x + 2.f; }(x); +} + +__device__ float d1(float x) { + return [](float x) { return x * 2.f; }(x); +} + +// DEVICE: amdgpu_kernel void @_Z2k0IZZ2f1PfENKUlS0_E_clES0_EUlfE_EvS0_T_( +template <typename F> +__global__ void k0(float *p, F f) { + p[0] = f(p[0]) + d0(p[1]) + d1(p[2]); +} + +void f0(float *p) { + [](float *p) { + *p = 1.f; + }(p); +} + +// The inner/outer lambdas are required to be mangled following ODR but their +// linkages are still required to keep the original `internal` linkage. + +// HOST: define internal void @_ZZ2f1PfENKUlS_E_clES_( +// DEVICE: define internal float @_ZZZ2f1PfENKUlS_E_clES_ENKUlfE_clEf( +void f1(float *p) { + [](float *p) { + k0<<<1,1>>>(p, [] __device__ (float x) { return x + 1.f; }); + }(p); +} +// HOST: @__hip_register_globals +// HOST: __hipRegisterFunction{{.*}}@_Z2k0IZZ2f1PfENKUlS0_E_clES0_EUlfE_EvS0_T_{{.*}}@0 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
