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

Fri Sep 30 09:23:37 PDT 2022

aaron.ballman added a comment.

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

> In D134456#3827377 <https://reviews.llvm.org/D134456#3827377>, @aaron.ballman wrote:
>
>> 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.

Heh, you interpreted me properly. :-D I mostly meant "does this attribute seem like it'd be usable in other circumstances or is it likely to be really specific to just this situation?

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

If "bad things" is "make the non-error path slow", my understanding is that's exactly their expectations.

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

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134456