[llvm-dev] The state of IRPGO (3 remaining work items)

[Sean Silva via llvm-dev]

On Mon, May 23, 2016 at 8:23 PM, Sean Silva <chisophugis at gmail.com> wrote:

> Jake and I have been integrating IRPGO on PS4, and we've identified 3
> remaining work items.
>
>
> - Driver changes
>
> 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. http://reviews.llvm.org/D15829) 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.
>
> I'd like to get consensus on a path forward.
> 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.
>
>
> - Pre-instrumentation passes
>
> 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
> http://reviews.llvm.org/D15828 did not improve over just inlining with
> threshold 100.
>
> (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)
>
> 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).
>
>
> - Warnings
>
> 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):
>
> 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).
>


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?

-- Sean Silva


>
> 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.
>
> 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.
>
> 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.
>
> 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.
>
> -- Sean Silva
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160525/14e678c2/attachment.html>