<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 25, 2015 at 11:17 AM, Paul Robinson <span dir="ltr"><<a href="mailto:Paul_Robinson@playstation.sony.com" target="_blank">Paul_Robinson@playstation.sony.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">================<br>
Comment at: include/llvm/CodeGen/CommandFlags.h:236<br>
@@ +235,3 @@<br>
+cl::opt<DebuggerKind><br>
+DebuggerTuning("debugger-tune",<br>
<span class="">+               cl::desc("Tune debug info for a particular debugger"),<br>
----------------<br>
</span>dblaikie wrote:<br>
> What's the command line argument for? I mean convenience of testing LLVM, a bit, but in general we don't want to use backend options like this when actually interacting with LLVM from Clang and other frontends. This should probably end up as a part of the CU debug info metadata? Or a module-level debug info metadata flag? (ala DWARF version)<br>
The command line option is precisely for testing; it is not how Clang communicates to LLVM.<br>
CommandFlags.h exists to define command-line options for the tools (llc, opt etc) only.  It does not define a command-line option that is accessible to Clang.<br>
Clang will set the TargetOptions field directly.<br></blockquote><div><br>Setting target options makes things a bit hard to test (it's hard to test that Clang does this correctly in a clang test - since it's just API, not part of the bitcode, etc) and difficult to deal with in LTO (the command line flag would be passed to the compilation, but needed by the linking step).<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
================<br>
Comment at: test/DebugInfo/X86/dwarf-public-names.ll:42<br>
@@ -41,3 +41,3 @@<br>
 ; NOPUB: debug_pubnames<br>
-; NOPUB: {{^$}}<br>
+; NOPUB-NEXT: {{^$}}<br>
<br>
----------------<br>
dblaikie wrote:<br>
> what happened to these two checks in this test?<br>
As originally written, this check does not fail if there is actually a debug_pubnames section, because it didn't insist that the empty line was immediately following the "debug_pubnames" header line.  Instead it would match the empty line at the end of the debug_pubnames dump, and pass.<br>
I made the same change on the LINUX path just for consistency.<br></blockquote><div><br>Can you make this change ahead of time to demonstrate that it's an independent/improvement change? (if you've already committed, no worries - something to consider for future commits)<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
This test update could actually be committed independently if you'd rather, as it's fixing a bug in the test rather than actually testing something I did in the rest of the patch.<br>
<div class="HOEnZb"><div class="h5"><br>
<a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D8506&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=FIetY5yH3d5uZV4U7WX6MSIYVQNLxmKRjL8rkFQzcIE&s=mCd_sMn1QQu1plcByKrCkfpnFsnVAf80Jimj82a9HG8&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/D8506</a><br>
<br>
EMAIL PREFERENCES<br>
  <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_settings_panel_emailpreferences_&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=FIetY5yH3d5uZV4U7WX6MSIYVQNLxmKRjL8rkFQzcIE&s=_yhHfhXJyPL4FwG-zSsY3WFiOfG9g9LyZ2-SQCRLDdw&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
<br>
<br>
</div></div></blockquote></div><br></div></div>