[clang] 589ce5f - [DebugInfo] Move constructor homing case in shouldOmitDefinition.
David Blaikie via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 31 13:22:14 PDT 2020
On Mon, Aug 24, 2020 at 9:32 PM Voss, Matthew via cfe-commits <
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> 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
> _______________________________________________
> cfe-commits mailing list
> 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/55390841/attachment.html>
More information about the cfe-commits
mailing list