<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal">Hi David and Amy,<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">This change works for PS4. Build + test passes.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Thanks,<o:p></o:p></p>
<p class="MsoNormal">Matthew<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b>From:</b> David Blaikie <dblaikie@gmail.com> <br>
<b>Sent:</b> Monday, August 31, 2020 1:22 PM<br>
<b>To:</b> Voss, Matthew <Matthew.Voss@sony.com><br>
<b>Cc:</b> Amy Huang <akhuang@google.com>; cfe-commits@lists.llvm.org<br>
<b>Subject:</b> Re: [clang] 589ce5f - [DebugInfo] Move constructor homing case in shouldOmitDefinition.<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">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:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal">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" target="_blank">http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/73945</a><o:p></o:p></p>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">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.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal"><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-" target="_blank">https://github.com/llvm/llvm-</a><br>
> project/commit/589ce5f7050dd83fd3f7dbc182ea0fb051ece994<br>
> DIFF: <a href="https://github.com/llvm/llvm-" 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" 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" 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" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><o:p></o:p></p>
</blockquote>
</div>
</div>
</div>
</div>
</body>
</html>