[PATCH] D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels

Mark de Wever via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 1 12:11:49 PDT 2020


Mordante added a comment.

In D86559#2250106 <https://reviews.llvm.org/D86559#2250106>, @staffantj wrote:

> In D86559#2250034 <https://reviews.llvm.org/D86559#2250034>, @aaron.ballman wrote:
>
>> In D86559#2243575 <https://reviews.llvm.org/D86559#2243575>, @staffantj wrote:
>>
>>> This is not a "teach everyone about it, it's safe" feature. It's there to produce a very fine-grained control in those cases where it really matters, and where profiling-guided optimizations would produce exactly the wrong result. Using this feature should be an automatic "is this needed" question in a code review. It is needed sometimes, just rarely.
>>
>> +1 but I would point out that when PGO is enabled, this attribute is ignored, so it's an either/or feature. Either you get tuned optimizations, or you get to guess at them, but the current design doesn't let you mix and match. Do see that as being a major issue with the design?
>
> We did have discussions in SG14 about the possible need for "inverse PGO" optimization passes, but we didn't have the compiler dev expertise to know if we were on a good track or not. From the 8 years I've spent working on this kind of code, I've never mixed the two, since the interactions are just too unpredictable. What I will frequently do however, is compare benchmarks between PGO generated code, and our hand-specified optimizations. Sometimes the PGO detects interesting program flow that a mere human brain could not have envisioned, which we can then take advantage of (or de-prioritize) as needed.

Are you currently using `__builtin_expect` and test whether these expectations match with the PGO expectations? This isn't part of the current patches, but it's something I intent to look at later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86559



More information about the cfe-commits mailing list