<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Aug 24, 2020 at 9:32 PM Voss, Matthew via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Amy,<br>
<br>
Looks like there's some test failures on the PS4 Linux bot as a result of this commit. Could you take a look? If the failure persists for a while, I may need to revert this to unclog the bots and our internal CI.<br>
<br>
<a href="http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/73945" rel="noreferrer" target="_blank">http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/73945</a></blockquote><div><br></div><div>Was this issue resolved? The patch was reverted and an update has been posted (& approved by me (though looks like it hasn't been recommitted yet)) - any chance Amy/Matthew/etc could check there's a good chance this issue is fixed by that update/won't regress on the recommit.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
<br>
Thanks,<br>
Matthew<br>
<br>
> -----Original Message-----<br>
> From: cfe-commits <<a href="mailto:cfe-commits-bounces@lists.llvm.org" target="_blank">cfe-commits-bounces@lists.llvm.org</a>> On Behalf Of Amy<br>
> Huang via cfe-commits<br>
> Sent: Monday, August 24, 2020 8:18 PM<br>
> To: <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
> Subject: [clang] 589ce5f - [DebugInfo] Move constructor homing case in<br>
> shouldOmitDefinition.<br>
> <br>
> <br>
> Author: Amy Huang<br>
> Date: 2020-08-24T20:17:59-07:00<br>
> New Revision: 589ce5f7050dd83fd3f7dbc182ea0fb051ece994<br>
> <br>
> URL: <a href="https://github.com/llvm/llvm-" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-</a><br>
> project/commit/589ce5f7050dd83fd3f7dbc182ea0fb051ece994<br>
> DIFF: <a href="https://github.com/llvm/llvm-" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-</a><br>
> project/commit/589ce5f7050dd83fd3f7dbc182ea0fb051ece994.diff<br>
> <br>
> LOG: [DebugInfo] Move constructor homing case in shouldOmitDefinition.<br>
> <br>
> For some reason the ctor homing case was before the template<br>
> specialization case, and could have returned false too early.<br>
> I moved the code out into a separate function to avoid this.<br>
> <br>
> Also added a run line to the template specialization test. I guess all the<br>
> -debug-info-kind=limited tests should still pass with =constructor, but<br>
> it's probably unnecessary to test for all of those.<br>
> <br>
> Differential Revision: <a href="https://reviews.llvm.org/D86491" rel="noreferrer" target="_blank">https://reviews.llvm.org/D86491</a><br>
> <br>
> Added:<br>
> <br>
> <br>
> Modified:<br>
>     clang/lib/CodeGen/CGDebugInfo.cpp<br>
>     clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp<br>
> <br>
> Removed:<br>
> <br>
> <br>
> <br>
> ##########################################################################<br>
> ######<br>
> diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp<br>
> b/clang/lib/CodeGen/CGDebugInfo.cpp<br>
> index e3442ecd4bd5..c2929d027a1b 100644<br>
> --- a/clang/lib/CodeGen/CGDebugInfo.cpp<br>
> +++ b/clang/lib/CodeGen/CGDebugInfo.cpp<br>
> @@ -2260,6 +2260,25 @@ static bool<br>
> hasExplicitMemberDefinition(CXXRecordDecl::method_iterator I,<br>
>    return false;<br>
>  }<br>
> <br>
> +static bool canUseCtorHoming(const CXXRecordDecl *RD) {<br>
> +  // Constructor homing can be used for classes that have at least one<br>
> +  // constructor and have no trivial or constexpr constructors.<br>
> +  // Skip this optimization if the class or any of its methods are<br>
> +marked<br>
> +  // dllimport.<br>
> +  if (RD->isLambda() || RD->hasConstexprNonCopyMoveConstructor() ||<br>
> +      isClassOrMethodDLLImport(RD))<br>
> +    return false;<br>
> +<br>
> +  if (RD->ctors().empty())<br>
> +    return false;<br>
> +<br>
> +  for (const auto *Ctor : RD->ctors())<br>
> +    if (Ctor->isTrivial() && !Ctor->isCopyOrMoveConstructor())<br>
> +      return false;<br>
> +<br>
> +  return true;<br>
> +}<br>
> +<br>
>  static bool shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,<br>
>                                   bool DebugTypeExtRefs, const RecordDecl<br>
> *RD,<br>
>                                   const LangOptions &LangOpts) { @@ -<br>
> 2294,23 +2313,6 @@ static bool<br>
> shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,<br>
>        !isClassOrMethodDLLImport(CXXDecl))<br>
>      return true;<br>
> <br>
> -  // In constructor debug mode, only emit debug info for a class when its<br>
> -  // constructor is emitted. Skip this optimization if the class or any<br>
> of<br>
> -  // its methods are marked dllimport.<br>
> -  //<br>
> -  // This applies to classes that don't have any trivial constructors and<br>
> have<br>
> -  // at least one constructor.<br>
> -  if (DebugKind == codegenoptions::DebugInfoConstructor &&<br>
> -      !CXXDecl->isLambda() && !CXXDecl-<br>
> >hasConstexprNonCopyMoveConstructor() &&<br>
> -      !isClassOrMethodDLLImport(CXXDecl)) {<br>
> -    if (CXXDecl->ctors().empty())<br>
> -      return false;<br>
> -    for (const auto *Ctor : CXXDecl->ctors())<br>
> -      if (Ctor->isTrivial() && !Ctor->isCopyOrMoveConstructor())<br>
> -        return false;<br>
> -    return true;<br>
> -  }<br>
> -<br>
>    TemplateSpecializationKind Spec = TSK_Undeclared;<br>
>    if (const auto *SD = dyn_cast<ClassTemplateSpecializationDecl>(RD))<br>
>      Spec = SD->getSpecializationKind(); @@ -2320,6 +2322,12 @@ static<br>
> bool shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,<br>
>                                    CXXDecl->method_end()))<br>
>      return true;<br>
> <br>
> +  // In constructor homing mode, only emit complete debug info for a<br>
> + class  // when its constructor is emitted.<br>
> +  if ((DebugKind != codegenoptions::DebugInfoConstructor) &&<br>
> +      canUseCtorHoming(CXXDecl))<br>
> +    return true;<br>
> +<br>
>    return false;<br>
>  }<br>
> <br>
> <br>
> diff  --git a/clang/test/CodeGenCXX/debug-info-template-explicit-<br>
> specialization.cpp b/clang/test/CodeGenCXX/debug-info-template-explicit-<br>
> specialization.cpp<br>
> index 4e41c4092bf4..ff0457e94404 100644<br>
> --- a/clang/test/CodeGenCXX/debug-info-template-explicit-<br>
> specialization.cpp<br>
> +++ b/clang/test/CodeGenCXX/debug-info-template-explicit-specialization.<br>
> +++ cpp<br>
> @@ -1,4 +1,7 @@<br>
> -// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -debug-info-<br>
> kind=limited %s -o - | FileCheck %s<br>
> +// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple<br>
> +-debug-info-kind=limited %s -o - | FileCheck %<br>
> +<br>
> +// Make sure this still works with constructor homing.<br>
> +// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple<br>
> +-debug-info-kind=constructor %s -o - | FileCheck %s<br>
> <br>
>  // Run again with -gline-tables-only or -gline-directives-only and verify<br>
> we don't crash.  We won't output  // type info at all.<br>
> <br>
> <br>
> <br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div>