[PATCH] D134456: [PGO] Consider parent context when weighing branches with likelyhood.

Anton Bikineev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 22 14:31:16 PDT 2022


AntonBikineev added a comment.

In D134456#3809581 <https://reviews.llvm.org/D134456#3809581>, @aaron.ballman wrote:

> I'm on the fence about this... the attributes were added as optimization hints, but for somewhat interesting use cases. The obvious one of "I think this path is likely/unlikely and so I'll help the compiler figure it out" was one such use case, but honestly that's akin to `inline` and `register` in terms of how likely it is the user will make a better decision than the optimizer. But the more interesting use case was "The optimizer is going to consider this to be the unlikely path, but I need it to be optimized because I need my failure code to fail as fast as possible". Basically, some folks wanted to use this to override the otherwise-reasonable decisions from the optimizer. Based on that, I'm not certain we should let PGO win -- the user is telling you "optimize based on something I know more about than you", and having a profile that covers the code path doesn't necessarily change that. WDYT?

I don't think that the second case is how things works today. PGO is still preferred over the attributes. However, the current behavior is somewhat strange:

- if the probability is infinitesimal (e.g. 0.00001%), the compiler will ignore the attribute,
- if the probability is zero (the branch never hit in the profile), the compiler will consider the attribute.



================
Comment at: clang/lib/CodeGen/CGStmt.cpp:815
   // Prefer the PGO based weights over the likelihood attribute.
   // When the build isn't optimized the metadata isn't used, so don't generate
----------------
rnk wrote:
> I lean towards implementing the intended behavior reflected in this comment here, which is that PGO data overrides programmer annotations in case of conflict. We should also check the PGO docs to see if this is documented. 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.
As Hans pointed out in a Chrome bug: the behaviour is documented here: https://clang.llvm.org/docs/AttributeReference.html#likely-and-unlikely:
"These attributes have no effect on the generated code when using PGO (Profile-Guided Optimization) or at optimization level 0."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134456



More information about the cfe-commits mailing list