r356598 - Recommit "Support attribute used in member funcs of class templates"

Thu Mar 28 13:26:53 PDT 2019

On Tue, Mar 26, 2019 at 7:40 PM Rafael Auler <rafaelauler at fb.com> wrote:

> I’m familiar with this issue as I had to fix a memory bug in LLVM IRLinker
> that was exposed by this commit. That’s why I initially reverted it.
> However, after fixing it, I was able to do the full clang LTO self-hosting
> with lld on Linux. Is there any way I can repro this issue? It’s probably a
> bad interaction of attribute used and an optimization. See
> https://reviews.llvm.org/D59552
>

Was this with full LTO or thin LTO?  I'm still working on getting a
reproducer.  This may be related to this issue:
https://bugs.llvm.org/show_bug.cgi?id=41236

- Michael Spencer

>
>
>
>
> *From: *Michael Spencer <bigcheesegs at gmail.com>
> *Date: *Tuesday, March 26, 2019 at 6:59 PM
> *To: *Rafael Auler <rafaelauler at fb.com>
> *Cc: *"cfe-commits at lists.llvm.org" <cfe-commits at lists.llvm.org>
> *Subject: *Re: r356598 - Recommit "Support attribute used in member funcs
> of class templates"
>
>
>
> On Wed, Mar 20, 2019 at 12:21 PM Rafael Auler via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
> Author: rafauler
> Date: Wed Mar 20 12:22:24 2019
> New Revision: 356598
>
> URL: http://llvm.org/viewvc/llvm-project?rev=356598&view=rev
> <https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D356598-26view-3Drev&d=DwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=kx31RNFp5lAJejEYwuEQ4Zc5A6GakBit07EY08bIAvc&m=Tp8AU6vZlFhDRTWeUHWkXzTHsspDHfWnErBtktRStLc&s=z5t_fduYusJn1DP4wsOLj9KYsPpKMyUkVXk2JAPRNM8&e=>
> Log:
> Recommit "Support attribute used in member funcs of class templates"
>
> This diff previously exposed a bug in LLVM's IRLinker, breaking
> buildbots that tried to self-host LLVM with monolithic LTO.
> The bug is now in LLVM by D59552
>
> Original commit message:
> As PR17480 describes, clang does not support the used attribute
> for member functions of class templates. This means that if the member
> function is not used, its definition is never instantiated. This patch
> changes clang to emit the definition if it has the used attribute.
>
> Test Plan: Added a testcase
>
> Reviewed By: aaron.ballman
>
> Differential Revision: https://reviews.llvm.org/D56928
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D56928&d=DwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=kx31RNFp5lAJejEYwuEQ4Zc5A6GakBit07EY08bIAvc&m=Tp8AU6vZlFhDRTWeUHWkXzTHsspDHfWnErBtktRStLc&s=Y54Z_weCYOa7tU_wNo9M6yMwDeLwDQKoNdrnZu_PllE&e=>
>
> Added:
>
> cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
> Modified:
>     cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=356598&r1=356597&r2=356598&view=diff
> <https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_lib_Sema_SemaTemplateInstantiateDecl.cpp-3Frev-3D356598-26r1-3D356597-26r2-3D356598-26view-3Ddiff&d=DwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=kx31RNFp5lAJejEYwuEQ4Zc5A6GakBit07EY08bIAvc&m=Tp8AU6vZlFhDRTWeUHWkXzTHsspDHfWnErBtktRStLc&s=Zh4mt8nrN7qZNDUMeoVkSQtZ2ebQTgXi_MOQa87S5fY&e=>
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Wed Mar 20 12:22:24
> 2019
> @@ -2232,6 +2232,20 @@ TemplateDeclInstantiator::VisitCXXMethod
>      Owner->addDecl(Method);
>    }
>
> +  // PR17480: Honor the used attribute to instantiate member function
> +  // definitions
> +  if (Method->hasAttr<UsedAttr>()) {
> +    if (const auto *A = dyn_cast<CXXRecordDecl>(Owner)) {
> +      SourceLocation Loc;
> +      if (const MemberSpecializationInfo *MSInfo =
> +              A->getMemberSpecializationInfo())
> +        Loc = MSInfo->getPointOfInstantiation();
> +      else if (const auto *Spec =
> dyn_cast<ClassTemplateSpecializationDecl>(A))
> +        Loc = Spec->getPointOfInstantiation();
> +      SemaRef.MarkFunctionReferenced(Loc, Method);
> +    }
> +  }
> +
>    return Method;
>  }
>
>
> Added:
> cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp?rev=356598&view=auto
> <https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_test_CodeGenCXX_attr-2Dused-2Dmember-2Dfunction-2Dimplicit-2Dinstantiation.cpp-3Frev-3D356598-26view-3Dauto&d=DwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=kx31RNFp5lAJejEYwuEQ4Zc5A6GakBit07EY08bIAvc&m=Tp8AU6vZlFhDRTWeUHWkXzTHsspDHfWnErBtktRStLc&s=CMIrOdMDTKRq5dy52CT5B9v2iMMA2dPslSvyhjaCb2o&e=>
>
> ==============================================================================
> ---
> cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
> (added)
> +++
> cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
> Wed Mar 20 12:22:24 2019
> @@ -0,0 +1,19 @@
> +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s |
> FileCheck %s
> +
> +// Check that PR17480 is fixed: __attribute__((used)) ignored in templated
> +// classes
> +namespace InstantiateUsedMemberDefinition {
> +template <typename T>
> +struct S {
> +  int __attribute__((used)) f() {
> +    return 0;
> +  }
> +};
> +
> +void test() {
> +  // Check that InstantiateUsedMemberDefinition::S<int>::f() is defined
> +  // as a result of the S class template implicit instantiation
> +  // CHECK: define linkonce_odr i32
> @_ZN31InstantiateUsedMemberDefinition1SIiE1fEv
> +  S<int> inst;
> +}
> +} // namespace InstantiateUsedMemberDefinition
>
>
>
> I believe this commit broke our (Apple) internal LTO bots.  I'm working on
> getting more information and narrowing it down.  The symptom is:
>
>
>
> Undefined symbols for architecture x86_64:
>
>   "llvm::cfg::Update<llvm::BasicBlock*>::dump() const", referenced from:
>
>       DominatorTreeBatchUpdates_LegalizeDomUpdates_Test::TestBody() in DominatorTreeBatchUpdatesTest.cpp.o
>
>       DominatorTreeBatchUpdates_LegalizePostDomUpdates_Test::TestBody() in DominatorTreeBatchUpdatesTest.cpp.o
>
>
>
> Has anyone else seen a similar issue with this commit?
>
> - Michael Spencer
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190328/c1b405e9/attachment-0001.html>