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

[Sean Silva via llvm-dev]

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).

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/20160523/c610c4c7/attachment.html>