llvm dev - May 2016 - The state of IRPGO (3 remaining work items)

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>

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.
>
Sean, thanks for the write up. It matches very well with what we think as
well.
>
>
> - 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.
>
>
I'd like to see IRPGO to be the default as well, but the first thing we
need is a driver level option to make the switch (prof-gen) -- currently we
rely on -Xclang option to switch between two modes, which is less than
ideal.

If the concern from Apple is that the old profile still need to work, then
this is problem already solved. The reason is that -fprofile-instr-use can
automatically detect the type of the profile and switch the mode.

>
> - 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).
>
>
>
Yes, Rong is re-collecting performance data before submitting the patch.

> - 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).
>
This can be enhanced with an user option to override the behavior. Can you
help filing a tracking bug?

>
> 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.
>
>
Rong has a patch addressing that -- will submit after cleanup pass change
is done.


> 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.
>
yes -- this is a common problem to other compilers as well.


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

> 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.
>
yes, this can be annoying. If the warnings can be buffered, then the
compiler can check if this is due to missing profile for the whole file and
can reduce the warnings into one single warning (source file has no profile
data).  Making it off by default sounds fine to me too if it is too noisy.

thanks,

David

>
> -- Sean Silva
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160523/20a313c3/attachment.html>

> On May 23, 2016, at 8:56 PM, Xinliang David Li <davidxl at
google.com> wrote:
> 
> 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.
> 
> Sean, thanks for the write up. It matches very well with what we think as
well.
+ 1

> - 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.
Sean pointed out the problematic scenario in D15829 (in plan "C"):

```
All existing user workflows continue to work, except for workflows that attempt
to llvm-profdata merge some old frontend profile data (e.g. they have checked-in
to version control and represents some special workload) with the profile data
from new binaries.
```

We can address this issue by (1) making sure llvm-profdata emits a helpful
warning when merging an FE-based profile with an IR-based one, and (2) keeping
an option to use FE instrumentation for PGO. Having (2) helps people who
can't (or don't want) to switch to IR PGO.

> 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'd really rather not introduce this inconsistency. I'm worried that it
might lead to Darwin becoming a second-tier platform for PGO.

Fred (CC'd) is following up with some of our internal users to check if we
can change the default behavior of -fprofile-instr-generate. He should be able
to chime in on this soon.

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

  * -fprofile-instr-generate: IR instrumentation
  * -fprofile-instr-generate=IR: IR instrumentation
  * -fprofile-instr-generate=FE: FE instrumentation
  * -fprofile-instr-generate -fcoverage-mapping: FE + coverage instrumentation

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.

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.

> 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.
> 
> I'd like to see IRPGO to be the default as well, but the first thing we
need is a driver level option to make the switch (prof-gen) -- currently we rely
on -Xclang option to switch between two modes, which is less than ideal.
> 
> If the concern from Apple is that the old profile still need to work, then
this is problem already solved. The reason is that -fprofile-instr-use can
automatically detect the type of the profile and switch the mode.
It's not just that. As Sean pointed out, we're concerned about old
profiles inter-operating poorly with new ones.

thanks,
vedant

> - 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/D15828did 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).
> 
> 
> 
> Yes, Rong is re-collecting performance data before submitting the patch.
>  
> - 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).
> 
> This can be enhanced with an user option to override the behavior. Can you
help filing a tracking bug?
>  
> 
> 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.
> 
> 
> Rong has a patch addressing that -- will submit after cleanup pass change
is done.
> 
>  
> 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.
> 
> yes -- this is a common problem to other compilers as well. 
> 
>  
> 
> 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.
> 
> 
> Right.
>  
> 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.
> 
> yes, this can be annoying. If the warnings can be buffered, then the
compiler can check if this is due to missing profile for the whole file and can
reduce the warnings into one single warning (source file has no profile data). 
Making it off by default sounds fine to me too if it is too noisy.
> 
> thanks,
> 
> David
>  
> 
> -- Sean Silva

On Mon, May 23, 2016 at 8:56 PM, Xinliang David Li <davidxl at google.com>
wrote:
>
>
> 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.
>>
>
> Sean, thanks for the write up. It matches very well with what we think as
> well.
>
>>
>>
>> - 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.
>>
>>
> I'd like to see IRPGO to be the default as well, but the first thing we
> need is a driver level option to make the switch (prof-gen) -- currently we
> rely on -Xclang option to switch between two modes, which is less than
> ideal.
>
> If the concern from Apple is that the old profile still need to work, then
> this is problem already solved. The reason is that -fprofile-instr-use can
> automatically detect the type of the profile and switch the mode.
>
Yes, since -fprofile-instr-use can automatically detect, the only issue is
merging with old profiles. The simplest solution is to add a driver flag
-fuse-the-old-pgo-because-i-want-to-merge-with-old-profiles (name is a
bikeshed) which temporarily allows users to continue to use FE PGO. I don't
see a reason why we would want FEPGO in the long term; we can always use
IRPGO with no pre-instrumentation passes to get the same effect.

>
>
>>
>> - 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).
>>
>>
>>
> Yes, Rong is re-collecting performance data before submitting the patch.
>
Great!

>
>
>> - 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).
>>
>
> This can be enhanced with an user option to override the behavior. Can you
> help filing a tracking bug?
>
Sure: https://llvm.org/bugs/show_bug.cgi?id=27867

>
>
>>
>> 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.
>>
>>
> Rong has a patch addressing that -- will submit after cleanup pass change
> is done.
>
Great!

>
>
>
>> 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.
>>
>
> yes -- this is a common problem to other compilers as well.
>
>
>
>>
>> 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.
>>
>>
> Right.
>
>
>> 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.
>>
>
> yes, this can be annoying. If the warnings can be buffered, then the
> compiler can check if this is due to missing profile for the whole file and
> can reduce the warnings into one single warning (source file has no profile
> data).  Making it off by default sounds fine to me too if it is too noisy.
>
That might work, but would be tricky since there might be linkonce_odr
functions also in the translation unit and so not all functions would be
missing profile counters.

-- Sean Silva

>
> thanks,
>
> David
>
>
>>
>> -- Sean Silva
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160524/4ff4c13f/attachment.html>

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>

On Wed, May 25, 2016 at 12:12 PM, Sean Silva <chisophugis at gmail.com>
wrote:
>
>
> 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?
>
Are you proposing always emitting the meta data for internal functions?

David


>
> -- 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/965f9377/attachment.html>