staffantj added a comment.
In D86559#2242586 <https://reviews.llvm.org/D86559#2242586>, @Mordante wrote:
>> That is exactly the behavior I am coming to believe we should follow. You
>> can either write it on a compound statement that is guarded by a flow
>> control decision (`if`/`else`/`while`) or you can write it on a label,
>> otherwise the attribute is parsed and ignored (with a diagnostic). This
>> feels like the right mixture of useful with understandable semantics so far,
>> but perhaps we'll find other examples that change our minds.
>>
>> The fallthrough behavior question has one more case we may want to think
>> about:
>>
>> switch (x) {
>> case 0:
>> case 1:
>> [[likely]] case 2: break;
>> [[unlikely]] default:
>> }
>>
>> Does this mark cases `0` and `1` as being likely or not? I could see users
>> wanting to use shorthand rather than repeat themselves on all the cases.
>> However, I'm also not certain whether there would be any performance impact
>> if we marked only `case 2` as likely and left cases `0` and `1` with default
>> likelihood. My gut feeling is that this should only mark `case 2`, but
>> others may have different views.
>
> Not according to the standard: "A path of execution includes a label if and
> only if it contains a jump to that label."
> However for an if statement I use 2 weights: 2000 for likely, 1 for unlikely.
> (These can be configured by a command-line option already used for
> `__builtin_expect`.)
> For a switch I intent to use 3 weights: 2000 for likely, (2000 + 1)/2 for no
> likelihood, 1 for unlikely. `__builtin_expect` doesn't have a 'neutral'
> values since in a switch you can only mark one branch as likely. Instead of
> adding a new option I picked the midpoint.
> So the weights in your example would be:
>
> > switch (x) {
> case 0: // 1000
> case 1: // 1000
> [[likely]] case 2: break; // 2000
> [[unlikely]] default: // 1
> }
>
> In this case the user could also write:
>
> > switch (x) {
> case 0: // 1000
> case 1: // 1000
> case 2: break; // 1000
> [[unlikely]] default: // 1
> }
>
> where the values 0, 1, 2 still would be more likely than the default
> statement. Obviously this will be documented when I implement the switch.
> How do you feel about this approach?
As one of the SG14 industry members driving this, I'm firmly in the camp that
this is what we're expecting. In the first case the 1/2 case are "neutral".
This is a very explicit, and local, marker. Anything else makes it so vague as
to be unusable for fine tuned code.
I should also make the point that we are not talking about a feature that is
expected, or indeed should be, used by anyone other than someone with an
exceedingly good understanding of what is going on. 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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86559/new/
https://reviews.llvm.org/D86559
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
