<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Apr 3, 2014 at 11:14 AM, Diego Novillo <span dir="ltr"><<a href="mailto:dnovillo@google.com" target="_blank">dnovillo@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">On Thu, Apr 3, 2014 at 2:04 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:<br>

><br>
>   Looks mostly fine. I'm concerned about a -R option affecting the debug info that ends up in the object file. I'm also concerned that the diagnostic quality will degrade if you build with no debug info enabled at all.<br>

>   It seems to me that if we can afford the IR size increase from `-dwarf-column-info`, we can afford the IR size increase from storing the frontend's notion of 'source location' into the IR, and that would allow us to do the right thing without `-g` and to deal properly with macro expansions etc. (It's only 32 bits per location...)<br>

<br>
</div>But we *do* enable line table information when -Rpass= is given. We<br>
also enable column information, so all we alter is that we'll be<br>
emitting dwarf LOCs in final codegen.<br>
<br>
If you don't think dwarf LOC generation is fine, we could do one of two things:<br>
<br>
1- We already have srcloc metadata. It would be a matter of generating<br>
them on every IR node when -Rpass= is used.<br>
2- If -Rpass= is used but we found it necessary to force line and<br>
column table generation, we could note that so that the code generator<br>
out of the back end does not emit DWARF LOCs. Eric, if this doesn't<br>
sound silly to you, how hard would it be?</blockquote><div><br></div><div>I think either of these could work. Option 1 seems cleaner to me, and has the advantage of preserving macro and inclusion information. We could probably piggyback on the debug location emission code to emit the !srcloc in the right places.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">
> ================<br>
> Comment at: lib/Frontend/CompilerInvocation.cpp:523<br>
> @@ -522,1 +522,3 @@<br>
><br>
> +  if (Arg *A = Args.getLastArg(OPT_Rpass_EQ)) {<br>
> +    StringRef Val = A->getValue();<br>
> ----------------<br>
> ... so we take the last -Rpass= option, rather than combining them? I find that unintuitive, FWIW, but maybe this is the right model.<br>
<br>
</div>I could combine them with '|'. Perhaps, that's a more intuitive model<br>
for the command line. How's that sound?</blockquote><div><br></div><div>Sounds good to me.</div></div></div></div>