<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Jun 26, 2016 at 10:21 PM, Dean Michael Berris <span dir="ltr"><<a href="mailto:dberris@google.com" target="_blank">dberris@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><br><div class="gmail_quote"><span><div dir="ltr">On Mon, Jun 27, 2016 at 2:53 PM Xinliang David Li <<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">There is some misunderstanding about the intention of this flag. The purpose of the flag is not to turn on profile instrumentation (which already has -fprofile-instr-generate or -fprofile-generate for it), but to select which instrumentors to use for PGO (IR or FE). I prefer fewer flags too, but sharing flags for completely different purpose does not seem like the right thing to do.</div><div dir="ltr"><div><br></div></div></blockquote><div><br></div></span><div>Ah, right. It does seem like I'm misunderstanding the intention for that flag.</div><div><br></div><div>FWIW, I think consolidating the flags can happen later on, when we have a better idea of how the different instrumentation pieces fit together.</div></div></div></blockquote><div><br></div><div><br></div><div>I hope so :) Note that PGO related instrumentation is not entirely the same as general profiling instrumentations. The former should also have a corresponding profile-use component in the compiler. Current PGO instrumentation has edge/block profiling and a value profiler component. The FE based edge/block profiler is also shared with coverage testing. </div><div><br></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_quote"><div> Right now IIUC the only other thing that might count as an alternate instrumentation implementation is XRay (and I'm not even sure they're mutually exclusive either, i.e. both the profiling and XRay instrumentation should be able to live together in the same binary).</div></div></div></blockquote><div><br></div><div>Sanitizers (asan, tsan, ubsan, msan) are all program instrumenters. Asan also supports a mode for coverage testing. Profile related, we also have esan (efficiency sanitizer) under development. The sanitizer options of course are unified in its own category. </div><div><br></div><div>David </div><div><br></div><div><br></div><div><br></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_quote"><div></div><div>So if it's just the two that will be interacting, then sharing flags doesn't seem worth it. But if I'm missing another, then the case for consolidating the flags becomes stronger.</div><div><br></div><div>Cheers</div><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div></div><div>David</div></div><div dir="ltr"><div><br><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Jun 26, 2016 at 9:39 PM, Dean Michael Berris <span dir="ltr"><<a href="mailto:dberris@google.com" target="_blank">dberris@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"><div class="gmail_quote"><span><div dir="ltr">On Fri, Jun 24, 2016 at 1:44 AM Eric Christopher via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div></span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br><br><div class="gmail_quote"><span><div dir="ltr">On Thu, Jun 2, 2016, 6:41 PM Xinliang David Li via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:</div></span><span><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"><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Sounds fine to me, though I am not a fan of using unstable in the option. I think a more meaningful way (that capture the essence of the difference) is the following naming:</div><div>1) FEPGO: -fprofile-instr-generate=source or -fprofile-instr-generate=region</div><div>2) IR: -fprofile-instr-generate=cfg or -fprofile-instr-generate=graph</div><div><br></div><div>Also since -fprofile-instr-generate= form is already used to specify raw profile path, we may need a different driver option. Alternatives include</div><div>1) -fprofile-instrument=<...> -- this maps directly to the cc1 option we have today</div><div>or</div><div>2) -fpgo-instr=<> -- suggested by Fred or</div><div>3) -fpgo-instr-method=<...></div><div></div></div></div></div></blockquote></span></div><span><div><br></div><div>Random bikeshedding. I like fprofile-instrument because it merges a lot of similar ideas behind instrumenting - and oddly enough what I was suggesting in the x-ray thread before seeing this. </div><div><br></div></span></blockquote><div><br></div><div>+1 to -fprofile-instrument=... (as someone working on the XRay stuff, I'd much rather have less flags, and consolidate a lot of these similar things into a more inclusive flag).<br></div><div><br></div><div>I would even make it shorter, and say -finstrument={profile-..., xray-...} so we can have multiple "namespaced" values for -finstrument=.</div><div><br></div><div>Just my A$0.02.</div></div></div>
</blockquote></div><br></div></div></div></blockquote></span></div></div>
</blockquote></div><br></div></div>