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

xbolva00 added a comment. What is the status of this patch? 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/mailma

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

aaron.ballman added a comment. In D86559#2371581 , @Mordante wrote: > In D86559#2371058 , @aaron.ballman > wrote: > >> In D86559#2369317 , @Mordante wrote: >> >>> Then me tr

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

Mordante marked 2 inline comments as done. Mordante added a comment. In D86559#2371058 , @aaron.ballman wrote: > In D86559#2369317 , @Mordante wrote: > >> Then me try to clear up the confusion. >> >>> However, I co

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

aaron.ballman added a comment. In D86559#2369317 , @Mordante wrote: > Then me try to clear up the confusion. > The parser first parses the `LabelDecl` and tries to attach the attributes to > this declaration. If that fails instead of issue a diagnostic fo

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

Mordante planned changes to this revision. Mordante marked 2 inline comments as done. Mordante added a comment. Then me try to clear up the confusion. The parser first parses the `LabelDecl` and tries to attach the attributes to this declaration. If that fails instead of issue a diagnostic for a

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

aaron.ballman added a comment. Given that some parts of this confused me, I'd like to make sure I'm clear on the approach you're taking. Based on the fact that labels are statements (defined as [stmt.label], in the statment production, etc), I would expect that attributes which appertain to lab

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

Mordante updated this revision to Diff 302103. Mordante added a comment. `Sema::ActOnLabelStmt` now processes the statement attributes placed on the `LabelDecl`. Returning an `AttributedStmt` from this function seems to work as intended. This changes the behaviour of `[[nomerge]]` being allowed

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

Mordante added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6454 +static bool validateLikelihoodAttr(Sema &S, Decl *D, const ParsedAttr &A) { + if (!isa(D)) { Mordante wrote: > aaron.ballman wrote: > > Mordante wrote: > > > aaron.ballman wrote:

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

Mordante planned changes to this revision. Mordante added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6454 +static bool validateLikelihoodAttr(Sema &S, Decl *D, const ParsedAttr &A) { + if (!isa(D)) { aaron.ballman wrote: > Mordante wrote: > >

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

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6454 +static bool validateLikelihoodAttr(Sema &S, Decl *D, const ParsedAttr &A) { + if (!isa(D)) { Mordante wrote: > aaron.ballman wrote: > > This is entering into somewhat novel

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

Mordante added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6454 +static bool validateLikelihoodAttr(Sema &S, Decl *D, const ParsedAttr &A) { + if (!isa(D)) { aaron.ballman wrote: > This is entering into somewhat novel territory for attributes,

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

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6454 +static bool validateLikelihoodAttr(Sema &S, Decl *D, const ParsedAttr &A) { + if (!isa(D)) { This is entering into somewhat novel territory for attributes, so some of this

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

Mordante added a comment. Friendly ping. 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-comm

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

Mordante updated this revision to Diff 290809. Mordante added a comment. Update the patch to match the behaviour where the attribute only affects the substatement of an if statement. This means the likelihood attributes on labels are accepted but are a no-op. Since they're a no-op the documentat

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

staffantj added a comment. In D86559#2250344 , @Mordante wrote: > In D86559#2250106 , @staffantj wrote: > >> In D86559#2250034 , @aaron.ballman >> wrote: >> >>> In D86559#22

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

Mordante added a comment. In D86559#2250106 , @staffantj wrote: > In D86559#2250034 , @aaron.ballman > wrote: > >> In D86559#2243575 , @staffantj >> wrote: >> >>> This is n

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

aaron.ballman added a comment. In D86559#2250106 , @staffantj wrote: > In D86559#2250034 , @aaron.ballman > wrote: > >> In D86559#2243575 , @staffantj >> wrote: >> >>> As o

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

staffantj added a comment. In D86559#2250034 , @aaron.ballman wrote: > In D86559#2243575 , @staffantj wrote: > >> As one of the SG14 industry members driving this, I'm firmly in the camp >> that this is what we're

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

aaron.ballman added a comment. In D86559#2243575 , @staffantj wrote: > 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,

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

Mordante added a comment. In D86559#2243583 , @staffantj wrote: > The use case for this becomes clearer when considering that the attribute > that will be used 95% of the time is [[unlikely]]. You may have a constructor > that you wish to ask the compile

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

Mordante added a comment. In D86559#2243575 , @staffantj wrote: > 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 l

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

Mordante added a comment. Herald added a subscriber: danielkiss. In D86559#2242713 , @aaron.ballman wrote: > In D86559#2242586 , @Mordante wrote: > >>> That is exactly the behavior I am coming to believe we should

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

staffantj added a comment. In D86559#2236931 , @Quuxplusone wrote: > This feels like the wrong approach to me... but I admit that I don't know > what the "right" approach might be. (I doubt any right approach exists.) > > if (ch == ' ') [[likely]] { >

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

staffantj added a comment. In D86559#2242586 , @Mordante wrote: > if(a) // Attribute not allowed on a declaration, > > [[likely]] int i = 5; // but I can't think about a good use-case > // for this code. > >

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

staffantj added a comment. In 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`

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

aaron.ballman added a comment. In 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`/`wh

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

Mordante planned changes to this revision. Mordante added a comment. > 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, otherw

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

aaron.ballman added a comment. In D86559#2239859 , @Mordante wrote: > In D86559#2239013 , @aaron.ballman > wrote: > >> I'd like to understand your reasoning for ignore + diagnose as opposed to >> count attrs (+ op

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

Mordante added a comment. In D86559#2239013 , @aaron.ballman wrote: > I'd like to understand your reasoning for ignore + diagnose as opposed to > count attrs (+ optionally diagnose) or other strategies. I can see pros and > cons to basically anything we

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

aaron.ballman added a comment. In D86559#2237037 , @Mordante wrote: > In D86559#2236931 , @Quuxplusone > wrote: > >> It seems like this patch would basically "copy" the `[[likely]]` attribute >> from line C up to

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

Mordante added a comment. In D86559#2236931 , @Quuxplusone wrote: > It seems like this patch would basically "copy" the `[[likely]]` attribute > from line C up to lines A and B, where it would reinforce the likelihood of > path A and (maybe?) "cancel out

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

Quuxplusone added a comment. This feels like the wrong approach to me... but I admit that I don't know what the "right" approach might be. (I doubt any right approach exists.) if (ch == ' ') [[likely]] { goto whitespace; // A } else if (ch == '\n' || ch == '\t') [[unlikely]] { g

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

Mordante created this revision. Mordante added reviewers: rsmith, rjmccall, aaron.ballman. Mordante added a project: clang. Herald added a subscriber: cfe-commits. Mordante requested review of this revision. Allows the likelihood attributes on label, which are not part of a case statement. When a