<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On May 25, 2016, at 4:34 PM, Sean Silva via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_extra"><br class="Apple-interchange-newline"><br class=""><div class="gmail_quote">On Wed, May 25, 2016 at 2:17 PM, Xinliang David Li<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a href="mailto:davidxl@google.com" target="_blank" class="">davidxl@google.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br 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;"><div dir="ltr" class=""><br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote"><span class="">On Wed, May 25, 2016 at 12:12 PM, Sean Silva<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a href="mailto:chisophugis@gmail.com" target="_blank" class="">chisophugis@gmail.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br 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;"><div dir="ltr" class=""><br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote"><span class="">On Mon, May 23, 2016 at 8:23 PM, Sean Silva<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a href="mailto:chisophugis@gmail.com" target="_blank" class="">chisophugis@gmail.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br class=""></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;"><div dir="ltr" class="">Jake and I have been integrating IRPGO on PS4, and we've identified 3 remaining work items.<div class=""><br class=""></div><div class=""><br class=""></div><div class="">- Driver changes</div><div class=""><br class=""></div><div class="">We'd like to make IRPGO the default on PS4. We also think that it would be beneficial to make IRPGO the default PGO on all platforms (coverage would continue to use FE instr as it does currently, of course). In previous conversations (e.g. <a href="http://reviews.llvm.org/D15829" target="_blank" class="">http://reviews.llvm.org/D15829</a>) it has come up that Apple have requirements that would prevent them from moving to IRPGO as the default PGO, at least without a deprecation period of one or two releases.</div><div class=""><br class=""></div><div class="">I'd like to get consensus on a path forward.</div><div class="">As a point of discussion, how about we make IRPGO the default on all platforms except Apple platforms. I really don't like fragmenting things like this (e.g. if a third-party tests "clang's" PGO they will get something different depending on the platform), but I don't see another way given Apple's constraints.</div><div class=""><br class=""></div><div class=""><br class=""></div><div class="">- Pre-instrumentation passes</div><div class=""><br class=""></div><div class="">Pre-instrumentation optimization has been critical for reducing the overhead of PGO for the PS4 games we tested (as expected). However, in our measurements (and we are glad to provide more info) the main benefit was inlining (also as expected). A simple pass of inlining at threshold 100 appeared to give all the benefits. Even inlining at threshold 0 gave almost all the benefits. For example, the passes initially proposed in <a href="http://reviews.llvm.org/D15828" target="_blank" class="">http://reviews.llvm.org/D15828</a><span class="Apple-converted-space"> </span>did not improve over just inlining with threshold 100.</div><div class=""><br class=""></div><div class="">(due to PR27299 we also need to add simplifycfg after inlining to clean up, but this doesn't affect the instrumentation overhead in our measurements)</div><div class=""><br class=""></div><div class="">Bottom line: for our use cases, inlining does all the work, but we're not opposed to having more passes, which might be beneficial for non-game workloads (which is most code).</div><div class=""><br class=""></div><div class=""><br class=""></div><div class="">- Warnings</div><div class=""><br class=""></div><div class="">We identified 3 classes of issues which manifest as spammy warnings when applying profile data with IRPGO (these affect FEPGO also I believe, but we looked in depth at IRPGO):</div><div class=""><br class=""></div><div class="">1. The main concerning one is that getPGOFuncName mangles the filename into the counter name. This causes us to get instrprof_error::unknown_function when the pgo-use build is done in a different build directory from the training build (which is a reasonable thing to support). In this situation, PGO data is useless for all `static` functions (and as a byproduct results in a huge volume of warnings).</div></div></blockquote><div class=""><br class=""></div><div class=""><br class=""></div></span><div class="">Rong, I was just looking at implementing a fix for this, but noticed something. Can we get rid of the "InLTO" argument to getPGOFuncName if we unconditionally apply the funcname metadata to all functions?</div></div></div></div></blockquote><div class=""><br class=""></div></span><div class="">Are you proposing always emitting the meta data for internal functions?</div></div></div></div></blockquote><div class=""><br class=""></div><div class="">Yes; actually for all functions. If I understand correctly, the InLTO argument fixes a specific case of a general problem: the PGOFuncName is currently constructed via an algorithm that depends on the current state of the module and the result can therefore change as the module is transformed.</div><div class="">To solve this, the idea is to only run the algorithm at a single point of truth (the module as seen by prof-gen/prof-use) and freeze the result in metadata. All other accesses to PGOFuncName simply read that metadata, so there is no possibility for getting out of date.</div><div class=""><br class=""></div><div class="">Currently, it seems the meaning of the InLTO flag is basically "we are accessing the PGOFuncName at a point that is not prof-gen/prof-use, so use the metadata". I just want to make that clearer.</div></div></div></div></div></blockquote><div><br class=""></div><div>To provide more details: specifically the transformations that require the metadata are the one that lead to changing the "PGOFuncName", so it is specifically 1) turning a symbol in to internal/private, and 2) straightforward renaming (which happens when you LTO-link two files that both have a local symbol with the same name).</div><div>Right now I believe the metadata is added specifically at the right place in the pipeline and only to cover the minimal amount of symbols that need to have the metadata (as David wrote, to avoid using extra memory).</div><div><br class=""></div><div><br class=""></div><div>-- </div><div>Mehdi</div><div><br class=""></div><div><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""><br class=""></div><div class="">-- Sean Silva</div><div class=""> </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;"><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><span class=""><font color="#888888" class=""><div class=""><br class=""></div><div class="">David</div></font></span><span class=""><div class=""><br class=""></div><div class=""> </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;"><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><span class=""><font color="#888888" class=""><div class=""><br class=""></div><div class="">-- Sean Silva</div></font></span><span class=""><div class=""> </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;"><div dir="ltr" class=""><div class=""><br class=""></div><div class="">2. In different TU's, pre-instr inlining might make different inlining decisions (for example, different functions may be available for inlining), causing hash mismatch errors (instrprof_error::hash_mismatch). In building a large game, we only saw 8 instance of this, so it is not as severe as 1, but would be good to fix.</div><div class=""><br class=""></div><div class="">3. A .cpp file may be compiled and put into an archive, but then not selected by the linker and will therefore not result in a counter in the profraw. When compiling this file with pgo-use, instrprof_error::unknown_function will result and a warning will be emitted.</div><div class=""><br class=""></div><div class="">Case 1 can be fixed using a function hash or other unique identifier instead of a file path. David, in D20195 you mentioned that Rong was working on a patch that would fix 2; we are looking forward to that.</div><div class=""><br class=""></div><div class="">For 3, I unfortunately do not know of any solution. I don't think there is a way for us to make this warning reliable in the face of this circumstance. So my conclusion is that instrprof_error::unknown_function at least must be defaulted to off unfortunately.</div><span class=""><font color="#888888" class=""><div class=""><div class=""><br class=""></div><div class="">-- Sean Silva</div></div></font></span></div></blockquote></span></div><br class=""></div></div></blockquote></span></div><br class=""></div></div></blockquote></div><br class=""></div></div><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">_______________________________________________</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">LLVM Developers mailing list</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class=""><a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.org</a></span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class=""><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a></span></div></blockquote></div><br class=""></body></html>