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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D134456#3827332 <https://reviews.llvm.org/D134456#3827332>, @dexonsmith wrote:

> In D134456#3827263 <https://reviews.llvm.org/D134456#3827263>, @aaron.ballman wrote:
>
>> My worry is that parallel attributes will be used as:
>>
>>   #if __has_cpp_attribute(clang::likely_but_honor_this_one)
>>   #define LIKELY [[clang::likely_but_honor_this_one]]
>>   #elif __has_cpp_attribute(likely)
>>   #define LIKELY [[likely]]
>>   #else
>>   #define LIKELY
>>   #endif
>
> To be clear, I was imagining:
>
>   if (always_false()) [[likely]] [[clang::nopgo]] {
>     // ...
>   }
>
> where ``nopgo`` might be independently useful for telling clang to ignore any collected PGO data when estimating branch weights in a particular control flow block.
>
> Some users might combine the two into a macro ("always ignore the profile when I say something is likely!"), but I don't think there'd be a cascade.

Ah, I see, thank you for clarifying! Does that seem like a generally useful attribute to you? (It seems like it could be, but this isn't my area of expertise.)

Another thought I had is: when PGO is enabled, can we diagnose when PGO countermands an explicit annotation? Something to at least alert the user "hey, look over here, this suggestion was wrong" so they have more of a chance of knowing something is up?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134456