<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 24, 2016 at 4:10 PM, Xinliang David Li <span dir="ltr"><<a href="mailto:davidxl@google.com" target="_blank">davidxl@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 dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Tue, May 24, 2016 at 3:50 PM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">Zooming into the command-line option bike-shed:<br>
<span><br>
> On 2016-May-24, at 15:41, Vedant Kumar via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
><br>
> At its core I don't think -fprofile-instr-generate *implies* FE-based instrumentation. So, I'd like to see the driver do this (on all platforms):<br>
><br>
>  * -fprofile-instr-generate: IR instrumentation<br>
>  * -fprofile-instr-generate=IR: IR instrumentation<br>
>  * -fprofile-instr-generate=FE: FE instrumentation<br>
>  * -fprofile-instr-generate -fcoverage-mapping: FE + coverage instrumentation<br>
<br>
</span>I feel like this would be simpler:<br>
  * -fcoverage-mapping: -fprofile-instr-generate=FE + coverage instrumentation<br>
<br>
Maybe there's a downside I'm not seeing though?<br></blockquote><div><br></div><div><br></div></span><div>I proposed the same change in proposal B in the review thread.</div><div><br></div><div><div>B. Proposed new behavior:</div><div> -fprofile-instr-generate turns on IR late instrumentation</div><div> -fcoverage-mapping turns on FE instrumentation and coverage-mapping</div><div> -fprofile-instr-generate -fcoverage-mapping result in compiler warning</div><div> -fprofile-instr-use=<> will automatically determine how to use the</div></div><div><br></div><div>The upside is that -fcoverage-mapping itself does not do anything by itself today. This change will simplify its usage (without user specifying -fprofile-instr-generate)</div><div><br></div><div>The downside Sean mentioned is that this changes the existing behavior of -fcoverage-mapping which can be a surprise to users (though I wonder why would a user depend on this old behavior).</div></div></div></div></blockquote><div><br></div><div>I didn't really intend to say that it was a "downside" per se. I was just pointing it out that it is a separable step and orthogonal to the discussion. I agree that it would simplify things for users; we already prohibit -fcoverage-mapping by itself, so we are strictly expanding what we accept.</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
Also, I don't like "FE".  Maybe "source"?  And instead of "IR", "llvm-ir" or something?<br></blockquote><div><br></div></span><div>Perhaps clang vs LLVM (similar to the cc1 option we have).  The downside is 'Clang' is clearly tied to Clang, but not any other FEs.</div><span class="HOEnZb"><font color="#888888"><div> </div><div><br></div><div>David</div></font></span><span class=""><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<span><br>
> It's a bit ugly because the meaning of -fprofile-instr-generate becomes context-sensitive. But, (1) it doesn't break existing common workflows and (2) it makes it easier to ship IRPGO. The big caveat here is that we'll need to wait a bit and see if our internal users are OK with this.<br>
><br>
> One alternative is to introduce a separate driver flag for IRPGO. This might not work well for Sony's existing users. I'd be interested in any feedback about this approach.<br>
<br>
</span>My first thought is `-mprofile-instr-generate`, since if it's not in the frontend then "-f" doesn't really make sense...</blockquote></span></div><br></div></div>
</blockquote></div><br></div></div>