[clang] 589ce5f - [DebugInfo] Move constructor homing case in shouldOmitDefinition.
Voss, Matthew via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 31 15:20:20 PDT 2020
Hi David and Amy,
This change works for PS4. Build + test passes.
Thanks,
Matthew
From: David Blaikie <dblaikie at gmail.com>
Sent: Monday, August 31, 2020 1:22 PM
To: Voss, Matthew <Matthew.Voss at sony.com>
Cc: Amy Huang <akhuang at google.com>; cfe-commits at lists.llvm.org
Subject: Re: [clang] 589ce5f - [DebugInfo] Move constructor homing case in shouldOmitDefinition.
On Mon, Aug 24, 2020 at 9:32 PM Voss, Matthew via cfe-commits <cfe-commits at lists.llvm.org<mailto:cfe-commits at lists.llvm.org>> wrote:
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
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.
Thanks,
Matthew
> -----Original Message-----
> From: cfe-commits <cfe-commits-bounces at lists.llvm.org<mailto: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<mailto: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<mailto:cfe-commits at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
cfe-commits at lists.llvm.org<mailto:cfe-commits at lists.llvm.org>
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200831/81b7192f/attachment-0001.html>
More information about the cfe-commits
mailing list