dexonsmith added a comment.

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

> In D134456#3827410 <https://reviews.llvm.org/D134456#3827410>, @dexonsmith 
> wrote:
>
>> In D134456#3827377 <https://reviews.llvm.org/D134456#3827377>, 
>> @aaron.ballman wrote:
>>
>>> 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?
>
> Hmmm, on the one hand, any optimization hint is really an advanced user 
> feature and so the opt remark might make sense. On the other hand, when I 
> posted about `[[likely]]` and PGO on Twitter (not exactly what I'd call a 
> scientific poll, take with an ocean of salt!), multiple people were surprised 
> and expected PGO to honor the attribute, which suggests users may want this 
> information. I don't have a good feel for what the user expectations are 
> here, so I think an opt remark would be the most conservative way to start, 
> and if we find that it's too hidden and non-expert users really need 
> warnings, we can add a warning at that point. WDYT?

Let's say it *was* scientific: even then, I disagree with your assertion that 
users *really* want this information, just because they were surprised about 
what happens. Many users will be surprised when told that the optimizer 
reorders their statements; or, more relevantly, that adding `inline` doesn't 
cause a function to be inlined; it doesn't follow that we should diagnose those 
situations.

I think what users want is good performance, and a way to fix the performance 
when it's poor. If optimization remarks aren't serving the purpose of the 
latter (which I believe was their design goal), maybe they can be improved 
somehow.

Optimization annotations are generally annoying for users since they give the 
illusion of control, and users feel frustrated when they aren't followed. 
Adding a diagnostic sounds nice, since then "at least they have some 
visibility"... but in practice the optimizer supersedes user expectations 
*everywhere*; you'll find it challenging to enable such diagnostics without 
getting back a wave of non-actionable diagnostics. IMO, `[[likely]]` isn't 
special here.

Another point: having a diagnostic fire (failing a `-Werror` build) depending 
on the content of the profile passed to `-fprofile-use` seems pretty hostile to 
build workflows.


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