dexonsmith added a comment.

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

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

Well, "generally useful" might be a stretch. Probably most users wouldn't 
want/need this. But if a user wants fine-grained control of the optimizer 
(probably not achievable, really), then turning off PGO seems like a knob they 
might want.

On the other hand, maybe `[[nopgo]]` is all that the safety-critical code 
should be using. IIRC, `[[likely]]` is really saying "hint: the other path is 
cold", and the optimizer pessimists the other path. Perhaps the safety-critical 
crowd just wants to turn off all pessimization; if so, they'll find that adding 
`[[likely]]` to the must-fail-quickly error path will do bad things to the 
non-error path.

> 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?

Seems pretty noisy, much like when the inliner countermands an `inline` hint. 
Maybe an optimization remark would be appropriate, allowing advanced users to 
selectively enable them in a critical section when debugging a performance 
issue?


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