aaron.ballman added a comment.

In D134456#3810769 <https://reviews.llvm.org/D134456#3810769>, 
@serge-sans-paille wrote:

> Shouldn't this be a situation where `-Wmisexpect` shold / could trigger?

If we're going to let PGO override what the user explicitly wrote in their 
code, we definitely should be diagnosing it. But I'm still not convinced that 
PGO winning actually matches the intent of the feature.

https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0479r2.html was the 
last version of the paper which still had its motivation section (<aside>it's 
very frustrating when WG21 authors strip relevant information from the paper 
being adopted!</aside>). It talks about PGO explicitly:

  Objection #2: Why should branch hints be standardized when many compilers 
implement PGO?
  
  Branch hints are definitely not meant to replace PGO; if PGO can be used it 
should be. PGO may be able to give the optimizer even more information then a 
branch hint. However, PGO and branch hints do not need to be mutually 
exclusive; branch hints could act as another indicator of expected frequent 
usage of particular branches.
  
  Also, PGO is not always easy to use. PGO necessitates creating a realistic 
test scenario to profile which, depending on the application, may be difficult 
and opens up the possibility that some important real world usage may be 
missed. The additional build tooling to keep the profile up to date, 
particularly across multiple OS's and compilers may also be challenging.
  
  Additionally, certain kinds of applications, such as those which may be 
distributed to clients who then build and run their own plugins in the 
distributed application may be very difficult or impossible to generate a 
realistic benchmark for, as there is no way of knowing how the client will use 
the application. Similar problems could also occur if an application is highly 
configurable; creating test scenarios and builds for all different possible 
configurations may not be realistic.
  
  Finally, PGO sometimes does the exact opposite of what is wanted for certain 
low-latency use cases. PGO tries to make the common case faster, but it may 
actually be more important to make the uncommon cases faster so that when they 
do occur they run as quickly as possible.

The first few paragraphs read to me like "sure, PGO should win" but the last 
paragraph reads like "no, PGO should lose". I remember when we were discussing 
this proposal in WG21 and the last paragraph was what motivated a number of 
committee members, especially in safety-critical spaces. Looking back through 
Core discussion, this point was brought up there as well as being important 
(https://lists.isocpp.org/core/2019/09/7193.php for those with access to the 
committee reflectors).

> If we've already made promises about how this is supposed to work, I'd prefer 
> to trust our past decisions rather than revisiting them.

I'm sympathetic to this but only if we actually made a decision here instead of 
documenting whatever behavior we got by default. 
https://reviews.llvm.org/D59467 is the original review which added this feature 
and from my reading of that we recognized the conflict between PGO and an 
explicit annotation, but it doesn't look like this concern was really 
considered.

Do users have some other escape hatch to tell PGO "ignore what you think you 
know about this branch" so that a user who *wants* PGO to lose has some ability 
to do that other than "don't use PGO"?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134456/new/

https://reviews.llvm.org/D134456

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to