[clang] 589ce5f - [DebugInfo] Move constructor homing case in shouldOmitDefinition.

[Voss, Matthew via cfe-commits]

Hi Amy,

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.

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/73945

Thanks,
Matthew

> -----Original Message-----
> From: cfe-commits <cfe-commits-bounces at lists.llvm.org> On Behalf Of Amy
> Huang via cfe-commits
> Sent: Monday, August 24, 2020 8:18 PM
> To: cfe-commits at lists.llvm.org
> Subject: [clang] 589ce5f - [DebugInfo] Move constructor homing case in
> shouldOmitDefinition.
> 
> 
> Author: Amy Huang
> Date: 2020-08-24T20:17:59-07:00
> New Revision: 589ce5f7050dd83fd3f7dbc182ea0fb051ece994
> 
> URL: https://github.com/llvm/llvm-
> project/commit/589ce5f7050dd83fd3f7dbc182ea0fb051ece994
> DIFF: https://github.com/llvm/llvm-
> project/commit/589ce5f7050dd83fd3f7dbc182ea0fb051ece994.diff
> 
> LOG: [DebugInfo] Move constructor homing case in shouldOmitDefinition.
> 
> For some reason the ctor homing case was before the template
> specialization case, and could have returned false too early.
> I moved the code out into a separate function to avoid this.
> 
> Also added a run line to the template specialization test. I guess all the
> -debug-info-kind=limited tests should still pass with =constructor, but
> it's probably unnecessary to test for all of those.
> 
> Differential Revision: https://reviews.llvm.org/D86491
> 
> Added:
> 
> 
> Modified:
>     clang/lib/CodeGen/CGDebugInfo.cpp
>     clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
> 
> Removed:
> 
> 
> 
> ##########################################################################
> ######
> diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp
> b/clang/lib/CodeGen/CGDebugInfo.cpp
> index e3442ecd4bd5..c2929d027a1b 100644
> --- a/clang/lib/CodeGen/CGDebugInfo.cpp
> +++ b/clang/lib/CodeGen/CGDebugInfo.cpp
> @@ -2260,6 +2260,25 @@ static bool
> hasExplicitMemberDefinition(CXXRecordDecl::method_iterator I,
>    return false;
>  }
> 
> +static bool canUseCtorHoming(const CXXRecordDecl *RD) {
> +  // Constructor homing can be used for classes that have at least one
> +  // constructor and have no trivial or constexpr constructors.
> +  // Skip this optimization if the class or any of its methods are
> +marked
> +  // dllimport.
> +  if (RD->isLambda() || RD->hasConstexprNonCopyMoveConstructor() ||
> +      isClassOrMethodDLLImport(RD))
> +    return false;
> +
> +  if (RD->ctors().empty())
> +    return false;
> +
> +  for (const auto *Ctor : RD->ctors())
> +    if (Ctor->isTrivial() && !Ctor->isCopyOrMoveConstructor())
> +      return false;
> +
> +  return true;
> +}
> +
>  static bool shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,
>                                   bool DebugTypeExtRefs, const RecordDecl
> *RD,
>                                   const LangOptions &LangOpts) { @@ -
> 2294,23 +2313,6 @@ static bool
> shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,
>        !isClassOrMethodDLLImport(CXXDecl))
>      return true;
> 
> -  // In constructor debug mode, only emit debug info for a class when its
> -  // constructor is emitted. Skip this optimization if the class or any
> of
> -  // its methods are marked dllimport.
> -  //
> -  // This applies to classes that don't have any trivial constructors and
> have
> -  // at least one constructor.
> -  if (DebugKind == codegenoptions::DebugInfoConstructor &&
> -      !CXXDecl->isLambda() && !CXXDecl-
> >hasConstexprNonCopyMoveConstructor() &&
> -      !isClassOrMethodDLLImport(CXXDecl)) {
> -    if (CXXDecl->ctors().empty())
> -      return false;
> -    for (const auto *Ctor : CXXDecl->ctors())
> -      if (Ctor->isTrivial() && !Ctor->isCopyOrMoveConstructor())
> -        return false;
> -    return true;
> -  }
> -
>    TemplateSpecializationKind Spec = TSK_Undeclared;
>    if (const auto *SD = dyn_cast<ClassTemplateSpecializationDecl>(RD))
>      Spec = SD->getSpecializationKind(); @@ -2320,6 +2322,12 @@ static
> bool shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,
>                                    CXXDecl->method_end()))
>      return true;
> 
> +  // In constructor homing mode, only emit complete debug info for a
> + class  // when its constructor is emitted.
> +  if ((DebugKind != codegenoptions::DebugInfoConstructor) &&
> +      canUseCtorHoming(CXXDecl))
> +    return true;
> +
>    return false;
>  }
> 
> 
> diff  --git a/clang/test/CodeGenCXX/debug-info-template-explicit-
> specialization.cpp b/clang/test/CodeGenCXX/debug-info-template-explicit-
> specialization.cpp
> index 4e41c4092bf4..ff0457e94404 100644
> --- a/clang/test/CodeGenCXX/debug-info-template-explicit-
> specialization.cpp
> +++ b/clang/test/CodeGenCXX/debug-info-template-explicit-specialization.
> +++ cpp
> @@ -1,4 +1,7 @@
> -// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -debug-info-
> kind=limited %s -o - | FileCheck %s
> +// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple
> +-debug-info-kind=limited %s -o - | FileCheck %
> +
> +// Make sure this still works with constructor homing.
> +// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple
> +-debug-info-kind=constructor %s -o - | FileCheck %s
> 
>  // Run again with -gline-tables-only or -gline-directives-only and verify
> we don't crash.  We won't output  // type info at all.
> 
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits