<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 14 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.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" style="margin-left:.5in">This seems pretty fine-grained for a CodeGenOpt (not that I've looked there, perhaps there are examples of similarly fine grained things already there?)- I'm curious to understand the preference towards that rather
than perhaps the more general "Debugger tuning" sort of thing Paul's implemented/could be pushed up here.<br>
<br>
<span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p></o:p></span></p>
<p class="MsoNormal"><a name="_MailEndCompose"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">On the LLVM side, debugger tuning is basically a way to package up settings for a variety of rather specific flags. Then the individual
flags control their respective fine-grained behaviors. This approach was very clearly favored during the whole "what is tuning" design discussion.<o:p></o:p></span></a></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">So, having an "emit explicit import" flag follows that same design decision and makes rather more sense than peppering IRGen with tuning or triple checks.
Whether the flag goes in CodeGenOpt or somewhere else is a separate question. There are other debug-related flags in there, but if they want to be factored out into their own DebugInfoOpt that's probably a separate topic/patch.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">--paulr<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></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 #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> David Blaikie [mailto:dblaikie@gmail.com]
<br>
<b>Sent:</b> Tuesday, September 08, 2015 7:36 PM<br>
<b>To:</b> reviews+D12624+public+25876849b7c59a9f@reviews.llvm.org; Richard Smith<br>
<b>Cc:</b> Romanova, Katya; Eric Christopher; Robinson, Paul; Adrian Prantl; cfe-commits<br>
<b>Subject:</b> Re: [PATCH] D12624: Top-level anonymous namespaces are missing import DW_TAG_imported_module and nested anonymous namespaces are not<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">On Tue, Sep 8, 2015 at 3:51 PM, Richard Smith via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<o:p></o:p></p>
<p class="MsoNormal">rsmith added inline comments.<br>
<br>
================<br>
Comment at: lib/CodeGen/CGDebugInfo.cpp:3263-3264<br>
@@ +3262,4 @@<br>
+ const NamespaceDecl *NSDecl = UD.getNominatedNamespace();<br>
+ if (!NSDecl->isAnonymousNamespace() ||<br>
+ CGM.getTarget().getTriple().isPS4CPU()) {<br>
+ DBuilder.createImportedModule(<br>
----------------<br>
probinson wrote:<br>
> rsmith wrote:<br>
> > I think we should do this unconditionally, to better match the source language semantics, but I'm curious what David, Eric, and other folks on the DWARF side think.<br>
> David (in previous discussions and review comments) has said he thinks it is unnecessary as the debugger already must know so much about C++ to get various things right, it might as well know that it has to implicitly import the anonymous namespace contents.
One example debugger UI allows the user to type source-like syntax, and requires the debugger to apply (for example) C++ parameter-type matching rules to distinguish between overloaded functions. Compared to this, implicit imports are child's play.<br>
><br>
> I believe Eric agrees with David; I don't remember whether Adrian said anything in the previous iterations of this patch.<br>
><br>
> I believe the explicit (although artificial) import is a good thing, because it matches the source language semantics. I find an important distinction between "which declarations are available in this scope" and "how does the user disambiguate declarations
in this scope." As a counterpart to the above debugger UI example, I postulate a GUI drop-down list of symbols available in-scope; this UI needs to know nothing about language semantics and automatic imports, if the DWARF provides the correct explicit import.
This suggests to me that the DWARF should provide it.<br>
><br>
> There's also the piddly detail that debuggers are not the only consumers of DWARF information, and presenting the DWARF in a more source-language-neutral way (i.e., with the explicit artificial import) could be beneficial for those other consumers, who might
not necessarily want to learn language-specific scoping rules.<br>
><br>
> No debugger will be thrown for a loop if it sees the explicit import; however for some debuggers it would be redundant (because they implicitly import the anonymous namespace already). There is a pretty trivial space savings if it's omitted.<br>
><br>
> Katya has mentioned the GCC and ICC precedent; in fairness I will say GCC didn't used to emit this, and GDB tolerated that.<br>
><br>
> Note that the DWARF standard does not tell us what to do; it merely tells us how to emit the import, if we want to emit one. Whether we want to emit one is up to us.<br>
><br>
I've chatted to David about this offline, and he said largely similar things. It seems that different DWARF consumers will want and expect different things here, so (sadly) we should do different things depending on who we think will be consuming the DWARF.<br>
<br>
I'm fine keeping this conditional, but I don't think IR generation should be making this decision based on the triple, so I'd prefer it was phrased in a different way: add a CodeGenOpt for whether to emit imports for anonymous namespaces, and enable it for
PS4 targets from the frontend.<o:p></o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">This seems pretty fine-grained for a CodeGenOpt (not that I've looked there, perhaps there are examples of similarly fine grained things already there?)- I'm curious to understand the preference towards that rather than perhaps the more
general "Debugger tuning" sort of thing Paul's implemented/could be pushed up here.<br>
<br>
- Dave<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">
<div>
<div>
<p class="MsoNormal"><br>
<br>
<a href="http://reviews.llvm.org/D12624" target="_blank">http://reviews.llvm.org/D12624</a><br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><o:p></o:p></p>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</div>
</div>
</body>
</html>