[Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt

[Mordante]

Summary

This contains the initial part of the implementation for the C++20 likelihood attributes.
For now it only handles them in an IfStmt, I want to add support for other statements after this one is done.

I was unsure whether it's preferred to have one patch for both the Sema and CodeGen part. If wanted I can split them easily.

[aaron.ballman]

Thank you for starting the implementation work on these attributes!

clang/include/clang/Basic/Attr.td

1292

Hmm, I'm on the fence about specifying 201803 for these attributes. Given that this is only the start of supporting the attribute, do we want to claim it already matches the standard's behavior? Or do we just want to return 1 to signify that we understand this attribute but we don't yet fully support it in common cases (such as on labels in switch statements, etc)?

As another question, should we consider adding a C2x spelling [[clang::likely]] and [[clang::unlikely]] to add this functionality to C?

clang/include/clang/Basic/AttrDocs.td

1698

Why? I would expect this to be reasonable code:

if (foo) [[likely]] {
  ...
} else if (bar) [[unlikely]] {
  ...
} else if (baz) [[likely]] {
  ...
} else {
  ...
}

Especially because I would expect this to be reasonable code:

switch (value) {
[[likely]] case 0: ... break;
[[unlikely]] case 1: ... break;
[[likely]] case 2: ... break;
[[unlikely]] default: ... break;
}

As motivating examples, consider a code generator that knows whether a particular branch is likely or not and it writes out the attribute on all branches. Or, something like this:

float rnd_value = get_super_random_number_between_zero_and_one();
if (rnd_value < .1) [[unlikely]] {
} else if (rnd_value > .9) [[unlikely]] {
} else [[likely]] {
}

1703

We should document whether we silently accept the attribute in other locations and do nothing with it (I'm not keen on this approach because it surprises users) or that we loudly diagnose the attribute in situations that we don't support it in (I think this is a more user-friendly approach, but may require a separate warning flag).

1709

Why does -Wimplicit-fallthrough matter?

clang/include/clang/Basic/DiagnosticGroups.td

659 ↗

(On Diff #282444)

Is this because you expect people will want to control diagnostics on if statements separate from diagnostics in other syntactic locations?

clang/include/clang/Basic/DiagnosticSemaKinds.td

2401

This isn't needed, we already have err_attributes_are_not_compatible.

2403

We've already got note_conflicting_attribute for this as well.

clang/lib/CodeGen/CGStmt.cpp

672

const auto *

693–694

I don't think the frontend should issue a diagnostic for that situation. I think codegen should handle chains appropriately instead as those seem plausible in reasonable code.

[• Quuxplusone]

clang/include/clang/Basic/AttrDocs.td

1698

Right, annotating both/multiple branches of a control statement should be perfectly fine. Even if (x) [[likely]] { } else [[likely]] { } should be perfectly okay as far as the code generator is concerned (and we should have a test for that); it's silly, but there's no reason to warn against it in the compiler docs.

Aaron, notice that if (x) [[likely]] { } else if (y) [[likely]] { } is not actually annotating "both" branches of any single if-statement. There you're annotating the true branch of an if (without annotating the else branch), and then annotating the true branch of another if (which doesn't have an else branch).

Mordante, in these docs, please document the "interesting" behavior of the standard attribute on labels — annotating a label is different from annotating the labeled statement itself. In particular,

[[likely]] case 1: case 2: foo(); break;
case 3: [[likely]] case 4: foo(); break;
case 5: case 6: [[likely]] foo(); break;

indicates that the likely cases are 1, 4, 5, and 6. (1 and 4 because their labels are annotated; 5 and 6 because their control flow passes through an annotated statement. Case 3's control flow passes through an annotated label, but that doesn't matter to the standard attribute.)

clang/include/clang/Basic/DiagnosticSemaKinds.td

2404

Typo: /declarered/declared/

clang/lib/Sema/SemaStmt.cpp

585

I agree with Aaron, this doesn't deserve a warning. Or at least there should be some thought put into the "threat model." (Are we protecting against someone typoing [[likely]]/[[likely]] when they meant [[likely]]/[[unlikely]]? But they weren't supposed to annotate both branches in the first place...)

clang/lib/Sema/SemaStmtAttr.cpp

349

Nit: Remove this blank line, for parallelism with lines 358-359.

clang/test/SemaCXX/attr-likelihood.cpp

102

I'd like to see a case like if (x) [[likely]] i=1; just to prove that it works on statements that aren't blocks or empty statements. (My understanding is that this should already work with your current patch.)

I'd like to see a case like if (x) { [[likely]] i=1; } to prove that it works on arbitrary statements. This should have the same effect as if (x) [[likely]] { i=1; }. My understanding is that your current patch doesn't get us there yet. If it's unclear how we'd get there by proceeding along your current trajectory, then I would question whether we want to commit to this trajectory at all, yet.

[Mordante]

clang/include/clang/Basic/Attr.td

1292

I was also somewhat on the fence, for now I'll change it to specify 1.

I'll have a look at the C2x changes. Just curious do you know whether there's a proposal to add this to C2x?

clang/include/clang/Basic/AttrDocs.td

1698

Aaron, the current CodeGen code for the switch statement allows all branches to have a weight, for the if there are only two weights allowed. As Arthur explained the chained ifs are different statements, so this will work properly.

Arthur, I agree we should add the documentation about the switch, but I'd rather do that when the attributes are implemented for the switch. For now I think it would make sense to add some documentation about the fact that the attribute can be placed inside the CompoundStmt.

1703

I'll update the documentation. I intend to continue to work on implementing these attributes so I feel adding a temporary diagnostic is not worth the effort.

1709

It doesn't, thanks for spotting the copy-paste error.

clang/include/clang/Basic/DiagnosticGroups.td

659 ↗

(On Diff #282444)

This is about warning about [[likely]] in both branches. Due to your an Arthur's remarks I've decided to remove them. Especially since Arthur correctly pointed out the attribute doesn't need to be directly after the if(x).

clang/include/clang/Basic/DiagnosticSemaKinds.td

2401

Thanks I didn't find them before, removed.

2403

Thanks I didn't find them before, removed.

clang/lib/CodeGen/CGStmt.cpp

693–694

This is the diagnostic about if(b) [[likely]] {...} else [[likely]] {...}. I'll remove the diagnostic, however the CodeGen will still prioritize the [[likely]] attribute in the ThenStmt. With Arthur's information the user can even do more unwise things:

if(b) [[likely]] {
    [[unlikely]] ...
} else [[likely]] {
    [[unlikely]]...
}

If the user provided conflicting attributes the CodeGen will not fail, but might not do what the user wanted.
This doesn't affect the chained if:

if(b) [[likely]] {
    ...
} else if(c) [[likely]] {
    ...
}

For the switch it will be allowed to annotate multiple cases with the same attribute and that should work as intended. (In a case it won't 'protect' against conflicting attributes in one case).

clang/lib/Sema/SemaStmt.cpp

585

Yes it protects against [[likely]]/[[likely]]. I've decided to remove the warning.

clang/test/SemaCXX/attr-likelihood.cpp

102

I agree it would be a good idea to add a test like if (x) [[likely]] i=1; but I don't feel adding an additional test in this file proves anything. I'll add a test to clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp to prove it not only accepts the code but also honours the attribute. This is especially important due to your correct observation that if (x) { [[likely]] i=1; } doesn't have any effect. The code is accepted but the CodeGen won't honour the attribute.

I think we can use the current approach, but I need to adjust getLikelihood. Instead of only testing whether the current Stmt is an AttributedStmt it needs to iterate over all Stmts and test them for the attribute. Obviously it should avoid looking into Stmts that also use the attribute, e.g:

if(b) {
    switch(c) {
        [[unlikely]] case 0: ... break; // The attribute "belongs" to the switch not to the if
        [[likely]] case 1: ... break; // The attribute "belongs" to the switch not to the if
    }
}

[aaron.ballman]

clang/include/clang/Basic/Attr.td

1292

I'll have a look at the C2x changes. Just curious do you know whether there's a proposal to add this to C2x?

There isn't one currently because there is no implementation experience with the attributes in C. This is a way to get that implementation experience so it's easier to propose the feature to WG14.

clang/include/clang/Basic/AttrDocs.td

1698

Aaron, notice that if (x) [[likely]] { } else if (y) [[likely]] { } is not actually annotating "both" branches of any single if-statement. There you're annotating the true branch of an if (without annotating the else branch), and then annotating the true branch of another if (which doesn't have an else branch).

I agree, which is why I pointed it out -- I think users are going to be surprised by this behavior because it won't do what they expect it to do.

As Arthur explained the chained ifs are different statements, so this will work properly.

We may have differing definitions of "properly." Users are going to expect to be able to write chains like this:

if (foo) [[likely]] {
} else if (bar) [[likely]] {
} else [[unlikely]] {
}

and have it behave as if they wrote this:

if (foo) [[likely]] {
} else [[likely]] if (bar) [[likely]] {
} else [[unlikely]] {
}

instead of this:

if (foo) [[likely]] {
} else [[unlikely]] if (bar) [[likely]] {
} else [[unlikely]] {
}

in terms of what information gets passed to the optimizer. Given what we currently can support for branch weights, Perhaps the behavior closest to the user's expectation would be as if they wrote this:

if (foo) {
} else if (bar) [[likely]] {
} else [[unlikely]] {
}

(So we say nothing about the first if/else decision branch because the user wants them both to be treated as equally likely.)

1703

SGTM!

clang/test/SemaCXX/attr-likelihood.cpp

102

This is especially important due to your correct observation that if (x) { [[likely]] i=1; } doesn't have any effect.

This code should diagnose the attribute as being ignored.

[Mordante]

clang/include/clang/Basic/Attr.td

1292

I was also somewhat on the fence, for now I'll change it to specify 1.

There seems to be an issue using the 1 so I used 2 instead. (Didn't want to look closely at the issue.)

1292

I'll have a look at the C2x changes. Just curious do you know whether there's a proposal to add this to C2x?

There isn't one currently because there is no implementation experience with the attributes in C. This is a way to get that implementation experience so it's easier to propose the feature to WG14.

Nice to know. It seems the C2x wasn't at straightforward as I hoped, so I didn't implement it yet. I intend to look at it later. I first want the initial part done to see whether this is the right approach.

clang/include/clang/Basic/AttrDocs.td

1698

Aaron, notice that if (x) [[likely]] { } else if (y) [[likely]] { } is not actually annotating "both" branches of any single if-statement. There you're annotating the true branch of an if (without annotating the else branch), and then annotating the true branch of another if (which doesn't have an else branch).

I agree, which is why I pointed it out -- I think users are going to be surprised by this behavior because it won't do what they expect it to do.

As Arthur explained the chained ifs are different statements, so this will work properly.

We may have differing definitions of "properly." Users are going to expect to be able to write chains like this:

if (foo) [[likely]] {
} else if (bar) [[likely]] {
} else [[unlikely]] {
}

and have it behave as if they wrote this:

if (foo) [[likely]] {
} else [[likely]] if (bar) [[likely]] {
} else [[unlikely]] {
}

instead of this:

if (foo) [[likely]] {
} else [[unlikely]] if (bar) [[likely]] {
} else [[unlikely]] {
}

in terms of what information gets passed to the optimizer. Given what we currently can support for branch weights, Perhaps the behavior closest to the user's expectation would be as if they wrote this:

if (foo) {
} else if (bar) [[likely]] {
} else [[unlikely]] {
}

(So we say nothing about the first if/else decision branch because the user wants them both to be treated as equally likely.)

1698

We may have differing definitions of "properly." Users are going to expect to be able to write chains like this:

if (foo) [[likely]] {
} else if (bar) [[likely]] {
} else [[unlikely]] {
}

and have it behave as if they wrote this:

if (foo) [[likely]] {
} else [[likely]] if (bar) [[likely]] {
} else [[unlikely]] {
}

instead of this:

if (foo) [[likely]] {
} else [[unlikely]] if (bar) [[likely]] {
} else [[unlikely]] {
}

I consider this the "proper" behaviour from the compiler's point of view. I think it's not intended to "taint" the entire path to the attribute. E.g

if(foo) { // 1
   // do stuff
   if(error) [[unlikely]] { // 2
       // handle error
   }
} else { // 3
   // do stuff     
}

Here I expect:

• 1 and 3 to be equally likely
• 2 to be unlikely

in terms of what information gets passed to the optimizer. Given what we currently can support for branch weights, Perhaps the behavior closest to the user's expectation would be as if they wrote this:

if (foo) {
} else if (bar) [[likely]] {
} else [[unlikely]] {
}

(So we say nothing about the first if/else decision branch because the user wants them both to be treated as equally likely.)

I'll give this some more thought and will write some more documentation on what my current approach will do and discuss it from there. It should also take the iteration statements into account and how they handle their likelihood. I'll also have a look at behaviour of GCC and MSVC to see how they've implemented the feature.

clang/test/SemaCXX/attr-likelihood.cpp

102

What I understood from Arthur and rereading the standard this should work, but the initial version of the patch didn't handle this properly.

[aaron.ballman]

clang/include/clang/Basic/Attr.td

1292

What issues are you running into? 1 is the default value when you don't specify a value specifically.

clang/include/clang/Basic/AttrDocs.td

1698

Here I expect:
1 and 3 to be equally likely
2 to be unlikely

That matches my expectations.

I'll give this some more thought and will write some more documentation on what my current approach will do and discuss it from there. It should also take the iteration statements into account and how they handle their likelihood. I'll also have a look at behaviour of GCC and MSVC to see how they've implemented the feature.

Thank you! I think my design principle for a binary decision is that repeating the attribute on both branches is valid but a noop because it gives the compiler no additional information, and using opposite attribute on the branches is identical to only one branch being annotated because the information given to the compiler is logically consistent.

clang/test/SemaCXX/attr-likelihood.cpp

102

What I understood from Arthur and rereading the standard this should work, but the initial version of the patch didn't handle this properly.

My original belief was that it's acceptable for the attribute to be placed there in terms of syntax, but the recommended practice doesn't apply to this case because there's no alternative paths of execution once you've entered the compound statement. That means:

if (x) [[likely]] { i = 1; }
// is not the same as
if (x) { [[likely]] i = 1; }

That said, I can squint at the words to get your interpretation and your interpretation matches what's in the original paper (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0479r2.html#examples).

Ultimately, I think we should strive for consistency between implementations, and I think we should file an issue with WG21 to clarify the wording once we figure out how implementations want to interpret questions like these.

[• Quuxplusone]

clang/test/SemaCXX/attr-likelihood.cpp

102

I don't know WG21's actual rationale, but I can imagine a programmer wanting to annotate something like this:

#define FATAL(msg) [[unlikely]] log_and_abort("Fatal error!", msg);
...
auto packet = receive();
if (packet.malformed()) {
    save_to_log(packet);
    FATAL("malformed packet");
}
...

The "absolute unlikeliness" of the malformed-packet codepath is encapsulated within the FATAL macro so that the programmer doesn't have to tediously repeat [[unlikely]] at some branch arbitrarily far above the FATAL point. Compilers actually do this already, every time they see a throw — every throw is implicitly "unlikely" and taints its whole codepath. We want to do something similar for [[unlikely]].

It's definitely going to be unsatisfyingly fuzzy logic, though, similar to how inlining heuristics and inline are today.

My understanding is that

if (x) { [[likely]] i = 1; }
if (x) [[likely]] { i = 1; }

have exactly the same surface reading: the same set of codepaths exist in both cases, and the same "x true, i gets 1" codepath is [[likely]] in both cases. Of course your heuristic might choose to treat them differently, just as you might choose to treat

struct S { int f() { ... } };
struct S { inline int f() { ... } };

differently for inlining purposes despite their identical surface readings.

[aaron.ballman]

clang/test/SemaCXX/attr-likelihood.cpp

102

Compilers actually do this already, every time they see a throw — every throw is implicitly "unlikely" and taints its whole codepath. We want to do something similar for [[unlikely]].

Heh, fun story, one of the use cases I was told was critical when discussing this feature in the committee came from the safety-critical space where they want to mark catch blocks as being likely in order to force the optimizer to do its best on the failure path, because that's the code path which had the hard requirements for bailing out quickly (so the elevator doesn't kill you, car doesn't crash, etc). They needed to be able to write:

try {
  ...
} catch (something& s) [[likely]] {
  ...
} catch (...) [[likely]] {
  ...
}

My understanding is that ... have exactly the same surface reading

I can read the standard to get to that understanding, but am skeptical that programmers will be able to predict what these attributes do anywhere they're written for anything but contrived examples with that interpretation. I think limiting the impact to only being at branch points is at least explicable behavior, otherwise programmers are left reasoning through code like:

auto what = []{ FATAL("malformed packet"); };
if (foo) {
  what();
} else {
  if (something) {
    what();
  }
}

(Does this make the foo branch unlikely? What impact is there on the else branch?) Now replace the lambda with a call to an inline function, or a GNU statement expression, an RAII constructor you can see the definition of, etc and ask the same questions.

My fear is that by going with the least restrictive design, programmers will eventually treat these attributes the same way they treat use of the register keyword -- something to be avoided because the best you can hope for is the implementation ignores it.

This isn't to say that I'm *opposed* to that approach, it's more that I'm skeptical of it unless we have some implementation agreement between major compilers about how to set user expectations appropriately.

[Mordante]

clang/include/clang/Basic/Attr.td

1292

It give the error "clang/include/clang/Basic/Attr.td:265:7: error: Standard attributes must have valid version information."

clang/test/SemaCXX/attr-likelihood.cpp

102

Compilers actually do this already, every time they see a throw — every throw is implicitly "unlikely" and taints its whole codepath. We want to do something similar for [[unlikely]].

Heh, fun story, one of the use cases I was told was critical when discussing this feature in the committee came from the safety-critical space where they want to mark catch blocks as being likely in order to force the optimizer to do its best on the failure path, because that's the code path which had the hard requirements for bailing out quickly (so the elevator doesn't kill you, car doesn't crash, etc). They needed to be able to write:

try {
  ...
} catch (something& s) [[likely]] {
  ...
} catch (...) [[likely]] {
  ...
}

Interesting use case, not sure how feasible it would be to implement it. I don't know how much it would affect the implementation if the exception handling. I tested with GCC and ICC and this doesn't compile. Looking at the standard it shouldn't compile. catch http://eel.is/c++draft/except.pre requires a compound statement http://eel.is/c++draft/stmt.block. (Obviously the attribute can be placed in the compound statement.)

A use-case I found where placing the attribute inside the compound statement is when using goto. I think this might be especially interesting when using it in C code.

int foo()
{
  int result = 0;
  struct bar *ptr;
  
  ptr = malloc(sizeof(struct bar));
  if(!ptr) {
      result = -ENOMEM;
      goto err;
  }

  ...
  if(!f(ptr)) {
    result = -ENOMDEV;
    goto err_free;
  }

[[unlikely]] err_free:
  free(ptr);  
[[unlikely]] err:
   return result; 
}

This allows to mark the error paths as [[unlikely]] by only placing the attribute on the label and every goto will mark the branch unlikely.

102

Ultimately, I think we should strive for consistency between implementations, and I think we should file an issue with WG21 to clarify the wording once we figure out how implementations want to interpret questions like these.

I agree having consistency would be a good. I did some testing.
It seems GCC's implementation is quite mature and does indeed handle the attributes inside the compound statements.
It seems MSVC's implementation seems a WIP.
It seems ICC accepts the attribute but it doesn't seem to change its branch weights.

What would be the best way to discuss this question with the other vendors? I assume I first need to write a paper, but would it then be best to discuss this with the other vendors or directly send it to the committee?

[Mordante]

clang/include/clang/Basic/Attr.td

1292

I'll have a look at the C2x changes. Just curious do you know whether there's a proposal to add this to C2x?

There isn't one currently because there is no implementation experience with the attributes in C. This is a way to get that implementation experience so it's easier to propose the feature to WG14.

Nice to know. It seems the C2x wasn't at straightforward as I hoped, so I didn't implement it yet. I intend to look at it later. I first want the initial part done to see whether this is the right approach.

I had another look and I got it working in C.

[Mordante]

Addresses review comments:

• Adds support for c2x using [[clang::likely]] and [[clang::unlikely]]
• Remove warnings about conflicting likelihoods, except those required by the Standard
• The attributes can now be placed inside the compound statement in of the ThenStmt and ElseStmt

The current implementation matches GCC's behaviour. This behaviour aligns best with my interpretation of the standard and GCC seems to have the most mature implementation. I would like to get the current implementation in Clang and proceed to work on the missing pieces. In parallel we can align with the other vendors and, if needed, adjust our implementation.

[aaron.ballman]

In D85091#2232588, @Mordante wrote:

The current implementation matches GCC's behaviour. This behaviour aligns best with my interpretation of the standard and GCC seems to have the most mature implementation. I would like to get the current implementation in Clang and proceed to work on the missing pieces. In parallel we can align with the other vendors and, if needed, adjust our implementation.

I agree with this direction.

clang/include/clang/Basic/Attr.td

1292

If you leave the version number off entirely, it defaults to 1.

clang/include/clang/Basic/AttrDocs.td

1692

I think this paragraph is misleading because the attribute no longer impacts the statement, it impacts the *entire branch the statement is contained within*.

1697

You should clarify here that you CAN annotate multiple statements in the same block and what we do about it. e.g.,

if (foo) {
  [[likely]];
  [[unlikely]];
  bar();
}

Note, I doubt this will happen as obviously as this example. More likely what will happen is:

#define ERROR_UNLESS(x) [[unlikely]] assert(x)

if (foo) {
  [[likely]];
  ERROR_UNLESS(baz == 12);
  bar();
}

1700

Spell out PGO and "or optimization" -> "or at optimization"

1727

Something else we should document is what our behavior is when the attribute is not immediately inside of an if or else statement. e.g.,

void func() { // Does not behave as though specified with [[gnu::hot]]
  [[likely]];
}

void func() {
  if (x) {
    {
      [[likely]]; // Does this make the if branch likely?
      SomeRAIIObj Obj;
    }
  } else {
  }
}

void func() {
  if (x) {
    int y = ({[[likely]]; /* Does this make the if branch likely? */ 1;});
  } else {
  }
}

clang/lib/CodeGen/CGStmt.cpp

656

//! -> //

691

Is early return the correct logic? Won't that give surprising results in the case the programmer has both attributes in the same compound statement? I think we need to look over all the statements in the current block, increment a counter when we hit [[likely]], decrement the counter when we hit [[unlikely]] and return whether the counter is 0, negative (unlikely), or positive (likely).

717

Perhaps not for this patch, but I wonder if we'd be doing users a good deed by alerting them when PGO weights do not match the attribute specified. e.g., if an attribute says "this branch is likely" and PGO shows it's unlikely, that seems like something the programmer may wish to know. WDYT?

723

I do not think conflicts should result in the first match being used (unless we're diagnosing the situation, at which point some of this code should be lifted into Sema and set a bit on an AST node that can be read during codegen time), so this comment may need to be updated.

733

LH = LH?

741

You should return None here.

clang/lib/Sema/SemaStmtAttr.cpp

345

I believe you can drop the calls to getSpelling() and just pass in the Attr* directly. The diagnostic engine takes care of printing the attribute properly.

clang/test/SemaCXX/attr-likelihood.cpp

102

What would be the best way to discuss this question with the other vendors? I assume I first need to write a paper, but would it then be best to discuss this with the other vendors or directly send it to the committee?

I would discuss directly with vendors first so that vendors can talk about what situations they are able to support and how, and then a paper could come second which details any suggested changes or open questions.

That said, for this particular patch, let's go with allowing it on arbitrary statements as GCC is doing. We can always revisit the decision later if we find an intractable problem.

[Mordante]

clang/include/clang/Basic/AttrDocs.td

1727

Something else we should document is what our behavior is when the attribute is not immediately inside of an if or else statement. e.g.,

void func() { // Does not behave as though specified with [[gnu::hot]]
  [[likely]];
}

No a few days ago I wondered whether it makes sense, but the [[gnu::hot]] should be at the declaration of the function and not in the body.
So I think this doesn't make sense and the [[likely]] here will be ignored.

void func() {

if (x) {
  {
    [[likely]]; // Does this make the if branch likely?
    SomeRAIIObj Obj;
  }
} else {
}

}

Yes this should work, the attribute will recursively visit compound statements.

void func() {

if (x) {
  int y = ({[[likely]]; /* Does this make the if branch likely? */ 1;});
} else {
}

}

Not in this patch. I'm working on more improvements to make switch statements working. I tested it with my current WIP and there it does work.
So in the future this will work.

clang/lib/CodeGen/CGStmt.cpp

691

Yes here it accepts the first likelihood it finds and accepts that as the wanted likelihood. I also start to doubt whether that's wanted. In my current switch WIP I issue an diagnostic if there's a conflict between the likelihood diagnostics and ignore them. I feel that's a better approach. This diagnostic is added to the -Wignored-attributes group, which is shown by default.

I don't like the idea of using a counter. If the attribute is "hidden" in a validation macro, adding it to a branch might suddenly change the likelihood of that branch.

717

I already investigated before and there's a diagnostic warn_profile_data_misexpect when using __builtin_expect so I expect I can reuse existing code. So I want to have a look at it, but I first want to get the more important parts of the likelihood attributes working properly.

723

Agreed. In my current switch WIP I already started experimenting with moving the likelihood selection to Sema and store the likelihood in the AST bits.

[aaron.ballman]

clang/include/clang/Basic/AttrDocs.td

1727

So I think this doesn't make sense and the [[likely]] here will be ignored.

Great, that matches the behavior I was hoping for.

Yes this should work, the attribute will recursively visit compound statements.

Great!

Not in this patch. I'm working on more improvements to make switch statements working.

I'm not certain what that example had to do with switch statement, but just as an FYI, I'd like this to work initially (assuming it's not overly hard) only because GNU statement expressions show up rather frequently in macros as an alternative form of the do ... while (0); pattern, which can have likely/unlikely path sensitivity.

clang/lib/CodeGen/CGStmt.cpp

691

I don't like the idea of using a counter. If the attribute is "hidden" in a validation macro, adding it to a branch might suddenly change the likelihood of that branch.

That's definitely true and is also a concern of mine. I don't know what the correct answer is here by going on a per-statement basis. None of the solutions are satisfying and all of them leave this feature with surprising behavior.

717

So I want to have a look at it, but I first want to get the more important parts of the likelihood attributes working properly.

SGTM

[Mordante]

I fixed the issues locally. Before I resubmit them I'd rather discuss in D86559 the direction we want to take, so we can avoid an additional review cycle.

clang/include/clang/Basic/Attr.td

1292

Yes and that gives the same error. So the default value doesn't "compile". I assume that's intentional to avoid setting a date of 1 for standard attributes. So we could keep it at 2 or switch back to 201803. Which value do you prefer?

clang/include/clang/Basic/AttrDocs.td

1692

I'll update the documentation including the other remarks added.

1727

It doesn't have anything to do with the switch by itself. However while working on the switch I needed a better way to follow the path of executed statements to handle break and return in a switch. As a side-effect it also handle the last example. So it will be possible to handle this case in a future patch. (However I think we should first discuss the approach to take in D86559.)

clang/lib/CodeGen/CGStmt.cpp

733

Has an extra LH =, thanks for catching it.

[aaron.ballman]

clang/include/clang/Basic/Attr.td

1292

Ah, yeah, you're right (sorry for the noise). I think 2 is fine for now unless you find that the new patch actually hits enough of the important cases from the standard to justify 201803. We can figure that out with the updated patch series.

clang/include/clang/Basic/AttrDocs.td

1727

Ah, thank you for the explanation.

[Mordante]

clang/include/clang/Basic/Attr.td

1292

For now let's keep it at 2, this patch won't implement the switch. Once the switch works the iteration statements still need to be done, but I think they aren't as important as the selection statements.

[Mordante]

Reworked the patch to match the behaviour as discussed in D86559.

• The likelihood attributes only have an effect when used directly on the ThenStmt and ElseStmt.
• Conflicting attributes on the ThenStmt and ElseStmt generate a diagnostic.
• Moved the likelihood determination from the CodeGen to the Sema. This requires the state to be stored in the AST.
• Updated the AST functions for the new likelihood bits.
• Updated the documentation.

Note currently there's no diagnostic for ignored attributes, this will be added in a future patch.

[aaron.ballman]

clang/include/clang/Basic/AttrDocs.td

1693

How about: This is done by marking the branch substatement with one of the two attributes.

1697

This sentence and the next one seem to say opposite things. I think this should read:

Annotating the 'true' and 'false' branch of an 'if' statement with the same likelihood attribute will result in a diagnostic and the attributes are ignored on both branches.

1705

How about: In Clang, the attributes will be ignored if they're not placed on the substatement of a selection statement.

1726

The formatting on this comment looks off (maybe different indentation, or tabs vs spaces).

1736

if or else statement.

clang/lib/AST/JSONNodeDumper.cpp

1452 ↗

(On Diff #288781)

I don't think this change should be necessary because the attribute should be written out for the AST node already. It's worth verifying though.

clang/lib/AST/TextNodeDumper.cpp

927 ↗

(On Diff #288781)

Similar here.

clang/lib/Sema/SemaStmt.cpp

618

returs -> returns

[Mordante]

Thanks again for your feedback. Once we agree on what to do with the Likelihood state in the output I'll finish the changes and submit a new patch.

clang/include/clang/Basic/AttrDocs.td

1697

I think the current wording is correct, but I like your wording better. So I'll use your wording.

1705

Your wording is correct for an if statement, but it will be different in a switch statement. I'll use your wording but replace selection statement with if or else statement. I like the substatement instead of first statement.

1726

Good catch, I thought I disabled using tabs.

clang/lib/AST/JSONNodeDumper.cpp

1452 ↗

(On Diff #288781)

They are in the AST. I added them to make it clear what the status is. I marked a case with a conflict, due to the same attribute in both branches. When the attribute is there but not on the substatement it's ignored. I'm not sure whether this is too important in the JSON output. I don't know who consumes it. So I don't feel to strongly about removing this from the JSON output.

What's your opinion?

clang/lib/AST/TextNodeDumper.cpp

927 ↗

(On Diff #288781)

I'm against removing it from the text output. This is be best way to determine how the attributes are parsed and processed. Especially when an attribute is ignored the likelihood may be different form what's expected. The Node->hasInitStorage()'s status is also visible in the AST and written out, so there are more examples of "duplicated" information.

Do you agree to keep it here?

clang/test/AST/ast-dump-if-json.cpp

1333 ↗

(On Diff #288781)

Here is the likelihood status in the If statement.

1429 ↗

(On Diff #288781)

The attribute in the ThenStmt.

1483 ↗

(On Diff #288781)

The attribute in the ElseStmt.

[rsmith]

Looking specifically for attributes in the 'then' and 'else' cases of an if seems like a fine first pass at this, but I think this is the wrong way to lower these attributes in the longer term: we should have a uniform treatment of them that looks for the most recent prior branch and annotates it with weights. We could do that either by generating LLVM intrinsic calls that an LLVM pass lowers, or by having the frontend look backwards for the most recent branch and annotate it. I suppose it's instructive to consider a case such as:

void mainloop() noexcept; // probably terminates by calling `exit`
void f() {
  mainloop();
  [[unlikely]];
  somethingelse();
}

... where the [[unlikely]]; should probably cause us to split the somethingelse() out into a separate basic block that we mark as cold, but should not cause f() itself or its call to mainloop() to be considered unlikely -- except in the case where mainloop() can be proven to always terminate, in which case the implication is that it's unlikely that f() is invoked. Cases like this probably need the LLVM intrinsic approach to model them properly.

clang/include/clang/AST/Stmt.h

175–178

Do we need to store this here? The "then" and "else" cases should be an AttributedStmt holding the likelihood attribute, so duplicating that info here seems redundant.

clang/lib/Sema/SemaStmt.cpp

578

Sema doesn't care about any of this; can you move this code to CodeGen and remove the code that stores likelihood data in the AST?

[aaron.ballman]

clang/lib/Sema/SemaStmt.cpp

578

FWIW, I asked for it to be moved out of CodeGen and into Sema because the original implementation in CodeGen was doing a fair amount of work trying to interrogate the AST for this information. Now that we've switched the design to only be on the substatement of an if/else statement (rather than an arbitrary statement), it may be that CodeGen is a better place for this again (and if so, I'm sorry for the churn).

[Mordante]

In D85091#2250657, @rsmith wrote:

Looking specifically for attributes in the 'then' and 'else' cases of an if seems like a fine first pass at this, but I think this is the wrong way to lower these attributes in the longer term: we should have a uniform treatment of them that looks for the most recent prior branch and annotates it with weights. We could do that either by generating LLVM intrinsic calls that an LLVM pass lowers, or by having the frontend look backwards for the most recent branch and annotate it. I suppose it's instructive to consider a case such as:

void mainloop() noexcept; // probably terminates by calling `exit`
void f() {
  mainloop();
  [[unlikely]];
  somethingelse();
}

... where the [[unlikely]]; should probably cause us to split the somethingelse() out into a separate basic block that we mark as cold, but should not cause f() itself or its call to mainloop() to be considered unlikely -- except in the case where mainloop() can be proven to always terminate, in which case the implication is that it's unlikely that f() is invoked. Cases like this probably need the LLVM intrinsic approach to model them properly.

We indeed considered similar cases and wondered whether it was really intended to work this way. Since this approach seems a bit hard to explain to users, I changed the code back to only allow the attribute on the substatement of the if and else. For now I first want to implement the simple approach, which I expect will be the most common use case. Once finished we can see what the next steps will be.

clang/include/clang/AST/Stmt.h

175–178

Agreed. I'll remove it again.

clang/lib/Sema/SemaStmt.cpp

578

At the moment the Sema cares about it to generate diagnostics about conflicting annotations:

if(i) [[ likely]] {}
else [[likely]] {}

This diagnostic only happens for an if statement, for a switch multiple values can be considered likely.
Do you prefer to have the diagnostic also in the CodeGen?

[rsmith]

clang/include/clang/AST/Stmt.h

1101–1107

I'd suggest you order these in increasing order of likeliness: unlikely, none, likely. Then you can determine what branch weights to use by a three-way comparison of the likeliness of the then branch and the likeliness of the else branch.

clang/lib/Sema/SemaStmt.cpp

578

It looked to me like you'd reached agreement to remove that diagnostic. Are you intending to keep it?

If so, then I'd suggest you make getLikelihood a member of Stmt so that CodeGen and the warning here can both easily call it.

[Mordante]

clang/lib/Sema/SemaStmt.cpp

578

@aaron.ballman I thought we wanted to keep this conflict warning, am I correct?
I'll add an extra function to the Stmt to test for a conflict and use that for the diagnostic in the Sema. This allows me to use LH_None for no attribute or a conflict of attributes in the CodeGen. Then there's no need for LH_Conflict and it can be removed from the enumerate.

[aaron.ballman]

clang/lib/Sema/SemaStmt.cpp

578

@aaron.ballman I thought we wanted to keep this conflict warning, am I correct?

I want to keep the property that any time the user writes an explicit annotation in their code but we decide to ignore the annotation (for whatever reason), the user gets some sort of diagnostic telling them their expectations may not be met. Because we're ignoring the attributes in the case where they're identical on both branches, I'd like to keep some form of diagnostic.

[aaron.ballman]

In D85091#2252632, @Mordante wrote:

In D85091#2250657, @rsmith wrote:

Looking specifically for attributes in the 'then' and 'else' cases of an if seems like a fine first pass at this, but I think this is the wrong way to lower these attributes in the longer term: we should have a uniform treatment of them that looks for the most recent prior branch and annotates it with weights. We could do that either by generating LLVM intrinsic calls that an LLVM pass lowers, or by having the frontend look backwards for the most recent branch and annotate it. I suppose it's instructive to consider a case such as:

void mainloop() noexcept; // probably terminates by calling `exit`
void f() {
  mainloop();
  [[unlikely]];
  somethingelse();
}

... where the [[unlikely]]; should probably cause us to split the somethingelse() out into a separate basic block that we mark as cold, but should not cause f() itself or its call to mainloop() to be considered unlikely -- except in the case where mainloop() can be proven to always terminate, in which case the implication is that it's unlikely that f() is invoked. Cases like this probably need the LLVM intrinsic approach to model them properly.

We indeed considered similar cases and wondered whether it was really intended to work this way. Since this approach seems a bit hard to explain to users, I changed the code back to only allow the attribute on the substatement of the if and else. For now I first want to implement the simple approach, which I expect will be the most common use case. Once finished we can see what the next steps will be.

+1 to the cautious approach. While I can understand how to implement what Richard is suggesting, I'm not convinced it results in readable code or that we're missing important optimization cases by using the more restrictive approach, so I'd like to get some field experience with users first.

[Mordante]

clang/lib/Sema/SemaStmt.cpp

578

Good then we're on the same page.

[Mordante]

Addresses review comments, mainly:

• Improving the documentation
• Removing the AST bits since they're no longer required

[aaron.ballman]

LGTM aside from a commenting nit. Thank you for the work on this!

clang/include/clang/AST/Stmt.h

1180

This only returns one value, so it can't return the likelihood of "the branches". How about something along the lines of:

returns the likelihood of the 'then' branch of an 'if' statement. The 'else' branch is required to determine whether both branches specify the same likelihood, which impacts the result.

[Mordante]

Thanks for your feedback during the review.

clang/include/clang/AST/Stmt.h

1180

Good catch, I'll use your suggestion.

[• Quuxplusone]

clang/include/clang/AST/Stmt.h

1180

s/impacts/affects/ ;)

[Mordante]

clang/include/clang/AST/Stmt.h

1180

Just in time before I commit this patch :-)

clang/include/clang/AST/Stmt.h

class IfStmtBitfields {
     unsigned HasElse : 1;
 
     /// True if this if statement has storage for a variable declaration.
     unsigned HasVar : 1;
 
     /// True if this if statement has storage for an init statement.
     unsigned HasInit : 1;
 
     /// The location of the "if".
     SourceLocation IfLoc;
   };
 
   class SwitchStmtBitfields {
     friend class SwitchStmt;
 
     unsigned : NumStmtBits;
 
     /// True if the SwitchStmt has storage for an init statement.
     unsigned HasInit : 1;
 
public:
   void operator delete(void *, void *) noexcept {}
 
 public:
   /// A placeholder type used to construct an empty shell of a
   /// type, that will be filled in later (e.g., by some
   /// de-serialization).
   struct EmptyShell {};
 
+  /// The likelihood of a branch being taken.
+  enum Likelihood {
+    LH_Unlikely = -1, ///< Branch has the [[unlikely]] attribute.
+    LH_None,          ///< No attribute set or branches of the IfStmt have
+                      ///< the same attribute.
+    LH_Likely         ///< Branch has the [[likely]] attribute.
+  };
+
 protected:
   /// Iterator for iterating over Stmt * arrays that contain only T *.
   ///
   /// This is needed because AST nodes use Stmt* arrays to store
   /// references to children (to be compatible with StmtIterator).
   template<typename T, typename TPtr = T *, typename StmtPtr = Stmt *>
   struct CastIterator
       : llvm::iterator_adaptor_base<CastIterator<T, TPtr, StmtPtr>, StmtPtr *,
public:
   SourceLocation getBeginLoc() const LLVM_READONLY;
   SourceLocation getEndLoc() const LLVM_READONLY;
 
   // global temp stats (until we have a per-module visitor)
   static void addStmtClass(const StmtClass s);
   static void EnableStatistics();
   static void PrintStats();
 
+  /// \returns the likelihood of a statement.
+  static Likelihood getLikelihood(const Stmt *S);
+
+  /// \returns the likelihood of the 'then' branch of an 'if' statement. The
+  /// 'else' branch is required to determine whether both branches specify the
+  /// same likelihood, which affects the result.
+  static Likelihood getLikelihood(const Stmt *Then, const Stmt *Else);
+
+  /// \returns whether the likelihood of the branches of an if statement are
+  /// conflicting. When the first element is \c true there's a conflict and
+  /// the Attr's are the conflicting attributes of the Then and Else Stmt.
+  static std::tuple<bool, const Attr *, const Attr *>
+  determineLikelihoodConflict(const Stmt *Then, const Stmt *Else);
+
   /// Dumps the specified AST fragment and all subtrees to
   /// \c llvm::errs().
   void dump() const;
   void dump(raw_ostream &OS, const ASTContext &Context) const;
 
   /// \return Unique reproducible object identifier
   int64_t getID(const ASTContext &Context) const;
 

clang/include/clang/Basic/Attr.td

 def FallThrough : StmtAttr {
   let Spellings = [CXX11<"", "fallthrough", 201603>,
                    C2x<"", "fallthrough", 201904>,
                    CXX11<"clang", "fallthrough">, GCC<"fallthrough">];
 //  let Subjects = [NullStmt];
   let Documentation = [FallthroughDocs];
 }
 
+def Likely : StmtAttr {
+  // FIXME: Change the date to 201803 once the implementation is finished.
+  let Spellings = [CXX11<"", "likely", 2>, C2x<"clang", "likely">];
+  let Documentation = [LikelihoodDocs];
+}
+
+def Unlikely : StmtAttr {
+  // FIXME: Change the date to 201803 once the implementation is finished.
+  let Spellings = [CXX11<"", "unlikely", 2>, C2x<"clang", "unlikely">];
+  let Documentation = [LikelihoodDocs];
+}
+
 def NoMerge : StmtAttr {
   let Spellings = [Clang<"nomerge">];
   let Documentation = [NoMergeDocs];
 }
 
 def FastCall : DeclOrTypeAttr {
   let Spellings = [GCC<"fastcall">, Keyword<"__fastcall">,
                    Keyword<"_fastcall">];

clang/include/clang/Basic/AttrDocs.td

[[clang::fallthrough]]; // warning: fallthrough annotation does not
                             //          directly precede case label
     q();
   case 77:  // warning: unannotated fall-through
     r();
   }
   }];
 }
 
+def LikelihoodDocs : Documentation {
+  let Category = DocCatStmt;
+  let Heading = "likely and unlikely";
+  let Content = [{
+The ``likely`` and ``unlikely`` attributes are used as compiler hints.
+The attributes are used to aid the compiler to determine which branch is
+likely or unlikely to be taken. This is done by marking the branch substatement
+with one of the two attributes.
+
+It isn't allowed to annotate a single statement with both ``likely`` and
+``unlikely``. Annotating the ``true`` and ``false`` branch of an ``if``
+statement with the same likelihood attribute will result in a diagnostic and
+the attributes are ignored on both branches.
+
+These attributes have no effect on the generated code when using
+PGO (Profile-Guided Optimization) or at optimization level 0.
+
+In Clang, the attributes will be ignored if they're not placed on the
+substatement of an ``if`` or ``else`` statement. The C++ Standard recommends
+to honor them on every statement in the path of execution, but that can be
+confusing:
+
+.. code-block:: c++
+
+  if (b) {
+    [[unlikely]] --b; // In the path of execution,
+                      // this branch is considered unlikely.
+  }
+
+  if (b) {
+    --b;
+    if(b)
+      return;
+    [[unlikely]] --b; // Not in the path of execution,
+  }                   // the branch has no likelihood information.
+
+  if (b) {
+    --b;
+    foo(b);
+    // Whether or not the next statement is in the path of execution depends
+    // on the declaration of foo():
+    // In the path of execution: void foo(int);
+    // Not in the path of execution: [[noreturn]] void foo(int);
+    // This means the likelihood of the branch depends on the declaration
+    // of foo().
+    [[unlikely]] --b;
+  }
+
+
+At the moment the attribute only has effect when used in an ``if`` or ``else``
+statement.
+
+.. code-block:: c++
+
+  if (b) [[likely]] { // Placement on the first statement in the branch.
+    // The compiler will optimize to execute the code here.
+  } else {
+  }
+
+  if (b)
+    [[unlikely]] b++; // Placement on the first statement in the branch.
+  else {
+    // The compiler will optimize to execute the code here.
+  }
+
+  if (b) {
+    [[unlikely]] b++; // Placement on the second statement in the branch.
+  }                   // The attribute will be ignored.
+
+  if (b) [[likely]] {
+    [[unlikely]] b++; // No contradiction since the second attribute
+  }                   // is ignored.
+
+  if (b)
+    ;
+  else [[likely]] {
+    // The compiler will optimize to execute the code here.
+  }
+
+  if (b)
+    ;
+  else
+    // The compiler will optimize to execute the next statement.
+    [[likely]] b = f();
+
+  if (b) [[likely]]; // Both branches are likely. A diagnostic is issued
+  else [[likely]];   // and the attributes are ignored.
+
+  if (b)
+    [[likely]] int i = 5; // Issues a diagnostic since the attribute
+                          // isn't allowed on a declaration.
+
+  }];
+}
+
 def ARMInterruptDocs : Documentation {
   let Category = DocCatFunction;
   let Heading = "interrupt (ARM)";
   let Content = [{
 Clang supports the GNU style ``__attribute__((interrupt("TYPE")))`` attribute on
 ARM targets. This attribute may be attached to a function definition and
 instructs the backend to generate appropriate function entry/exit code so that
 it can be used directly as an interrupt service routine.

clang/include/clang/Basic/DiagnosticSemaKinds.td

 def warn_final_dtor_non_final_class : Warning<
   "class with destructor marked '%select{final|sealed}0' cannot be inherited from">,
   InGroup<FinalDtorNonFinalClass>;
 def note_final_dtor_non_final_class_silence : Note<
   "mark %0 as '%select{final|sealed}1' to silence this warning">;
 
 // C++11 attributes
 def err_repeat_attribute : Error<"%0 attribute cannot be repeated">;
 
 // C++11 final
 def err_final_function_overridden : Error<
   "declaration of %0 overrides a '%select{final|sealed}1' function">;
 
 // C++11 scoped enumerations
 def err_enum_invalid_underlying : Error<
   "non-integral type %0 is an invalid underlying type">;
 def err_enumerator_too_large : Error<
   "enumerator value is not representable in the underlying type %0">;
 def ext_enumerator_too_large : Extension<
   "enumerator value is not representable in the underlying type %0">,
def warn_attribute_ignored_on_inline :
   Warning<"%0 attribute ignored on inline function">,
   InGroup<IgnoredAttributes>;
 def warn_nocf_check_attribute_ignored :
   Warning<"'nocf_check' attribute ignored; use -fcf-protection to enable the attribute">,
   InGroup<IgnoredAttributes>;
 def warn_attribute_after_definition_ignored : Warning<
   "attribute %0 after definition is ignored">,
    InGroup<IgnoredAttributes>;
+def warn_attributes_likelihood_ifstmt_conflict
+    : Warning<"conflicting attributes %0 are ignored">,
+      InGroup<IgnoredAttributes>;
 def warn_cxx11_gnu_attribute_on_type : Warning<
   "attribute %0 ignored, because it cannot be applied to a type">,
   InGroup<IgnoredAttributes>;
 def warn_unhandled_ms_attribute_ignored : Warning<
   "__declspec attribute %0 is not supported">,
   InGroup<IgnoredAttributes>;
 def err_decl_attribute_invalid_on_stmt : Error<
   "%0 attribute cannot be applied to a statement">;

clang/lib/AST/Stmt.cpp

 //===- Stmt.cpp - Statement AST Node Implementation -----------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the Stmt class and statement subclasses.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/AST/Stmt.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTDiagnostic.h"
+#include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclGroup.h"
 #include "clang/AST/Expr.h"
-#include "clang/AST/ExprConcepts.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/ExprConcepts.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ExprOpenMP.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/Token.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cassert>
 #include <cstring>
 #include <string>
-#include <utility>
 #include <type_traits>
+#include <utility>
 
 using namespace clang;
 
 static struct StmtClassNameTable {
   const char *Name;
   unsigned Counter;
   unsigned Size;
 } StmtClassInfo[Stmt::lastStmtConstant+1];
void Stmt::addStmtClass(StmtClass s) {
   ++getStmtInfoTableEntry(s).Counter;
 }
 
 bool Stmt::StatisticsEnabled = false;
 void Stmt::EnableStatistics() {
   StatisticsEnabled = true;
 }
 
+static std::pair<Stmt::Likelihood, const Attr *> getLikelihood(const Stmt *S) {
+  if (const auto *AS = dyn_cast_or_null<AttributedStmt>(S))
+    for (const auto *A : AS->getAttrs()) {
+      if (isa<LikelyAttr>(A))
+        return std::make_pair(Stmt::LH_Likely, A);
+
+      if (isa<UnlikelyAttr>(A))
+        return std::make_pair(Stmt::LH_Unlikely, A);
+    }
+
+  return std::make_pair(Stmt::LH_None, nullptr);
+}
+
+Stmt::Likelihood Stmt::getLikelihood(const Stmt *S) {
+  return ::getLikelihood(S).first;
+}
+
+Stmt::Likelihood Stmt::getLikelihood(const Stmt *Then, const Stmt *Else) {
+  Likelihood LHT = ::getLikelihood(Then).first;
+  Likelihood LHE = ::getLikelihood(Else).first;
+  if (LHE == LH_None)
+    return LHT;
+
+  // If the same attribute is used on both branches there's a conflict.
+  if (LHT == LHE)
+    return LH_None;
+
+  if (LHT != LH_None)
+    return LHT;
+
+  // Invert the value of Else to get the value for Then.
+  return LHE == LH_Likely ? LH_Unlikely : LH_Likely;
+}
+
+std::tuple<bool, const Attr *, const Attr *>
+Stmt::determineLikelihoodConflict(const Stmt *Then, const Stmt *Else) {
+  std::pair<Likelihood, const Attr *> LHT = ::getLikelihood(Then);
+  std::pair<Likelihood, const Attr *> LHE = ::getLikelihood(Else);
+  // If the same attribute is used on both branches there's a conflict.
+  if (LHT.first != LH_None && LHT.first == LHE.first)
+    return std::make_tuple(true, LHT.second, LHE.second);
+
+  return std::make_tuple(false, nullptr, nullptr);
+}
+
 /// Skip no-op (attributed, compound) container stmts and skip captured
 /// stmt at the top, if \a IgnoreCaptured is true.
 Stmt *Stmt::IgnoreContainers(bool IgnoreCaptured) {
   Stmt *S = this;
   if (IgnoreCaptured)
     if (auto CapS = dyn_cast_or_null<CapturedStmt>(S))
       S = CapS->getCapturedStmt();
   while (true) {

clang/lib/CodeGen/CGStmt.cpp

 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/Support/SaveAndRestore.h"
+#include "llvm/Transforms/Scalar/LowerExpectIntrinsic.h"
 
 using namespace clang;
 using namespace CodeGen;
 
 //===----------------------------------------------------------------------===//
 //                              Statement Emission
 //===----------------------------------------------------------------------===//
 
void CodeGenFunction::EmitIndirectGotoStmt(const IndirectGotoStmt &S) {
   llvm::BasicBlock *IndGotoBB = GetIndirectGotoBlock();
 
   // The first instruction in the block has to be the PHI for the switch dest,
   // add an entry for this branch.
   cast<llvm::PHINode>(IndGotoBB->begin())->addIncoming(V, CurBB);
 
   EmitBranch(IndGotoBB);
 }
+static Optional<std::pair<uint32_t, uint32_t>>
+getLikelihoodWeights(const IfStmt &If) {
+  switch (Stmt::getLikelihood(If.getThen(), If.getElse())) {
+  case Stmt::LH_Unlikely:
+    return std::pair<uint32_t, uint32_t>(llvm::UnlikelyBranchWeight,
+                                         llvm::LikelyBranchWeight);
+  case Stmt::LH_None:
+    return None;
+  case Stmt::LH_Likely:
+    return std::pair<uint32_t, uint32_t>(llvm::LikelyBranchWeight,
+                                         llvm::UnlikelyBranchWeight);
+  }
+  llvm_unreachable("Unknown Likelihood");
+}
 
 void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
   // C99 6.8.4.1: The first substatement is executed if the expression compares
   // unequal to 0.  The condition must be a scalar type.
   LexicalScope ConditionScope(*this, S.getCond()->getSourceRange());
 
   if (S.getInit())
     EmitStmt(S.getInit());
 
   if (S.getConditionVariable())
     EmitDecl(*S.getConditionVariable());
 
   // If the condition constant folds and can be elided, try to avoid emitting
   // the condition and the dead arm of the if/else.
   bool CondConstant;
   if (ConstantFoldsToSimpleInteger(S.getCond(), CondConstant,
                                    S.isConstexpr())) {
     // Figure out which block (then or else) is executed.
     const Stmt *Executed = S.getThen();
     const Stmt *Skipped  = S.getElse();
     if (!CondConstant)  // Condition false?
       std::swap(Executed, Skipped);
 
     // If the skipped block has no labels in it, just emit the executed block.
     // This avoids emitting dead code and simplifies the CFG substantially.
     if (S.isConstexpr() || !ContainsLabel(Skipped)) {
       if (CondConstant)
         incrementProfileCounter(&S);
       if (Executed) {
         RunCleanupsScope ExecutedScope(*this);
         EmitStmt(Executed);
       }
       return;
     }
   }
 
   // Otherwise, the condition did not fold, or we couldn't elide it.  Just emit
   // the conditional branch.
   llvm::BasicBlock *ThenBlock = createBasicBlock("if.then");
   llvm::BasicBlock *ContBlock = createBasicBlock("if.end");
   llvm::BasicBlock *ElseBlock = ContBlock;
   if (S.getElse())
     ElseBlock = createBasicBlock("if.else");
 
-  EmitBranchOnBoolExpr(S.getCond(), ThenBlock, ElseBlock,
-                       getProfileCount(S.getThen()));
+  // Prefer the PGO based weights over the likelihood attribute.
+  // When the build isn't optimized the metadata isn't used, so don't generate
+  // it.
+  llvm::MDNode *Weights = nullptr;
+  uint64_t Count = getProfileCount(S.getThen());
+  if (!Count && CGM.getCodeGenOpts().OptimizationLevel) {
+    Optional<std::pair<uint32_t, uint32_t>> LHW = getLikelihoodWeights(S);
+    if (LHW) {
+      llvm::MDBuilder MDHelper(CGM.getLLVMContext());
+      Weights = MDHelper.createBranchWeights(LHW->first, LHW->second);
+    }
+  }
+
+  EmitBranchOnBoolExpr(S.getCond(), ThenBlock, ElseBlock, Count, Weights);
 
   // Emit the 'then' code.
   EmitBlock(ThenBlock);
   incrementProfileCounter(&S);
   {
     RunCleanupsScope ThenScope(*this);
     EmitStmt(S.getThen());
   }
   EmitBranch(ContBlock);
 
   // Emit the 'else' code if present.
   if (const Stmt *Else = S.getElse()) {
     {
       // There is no need to emit line number for an unconditional branch.
       auto NL = ApplyDebugLocation::CreateEmpty(*this);
       EmitBlock(ElseBlock);
     }
     {
       RunCleanupsScope ElseScope(*this);
       EmitStmt(Else);
     }
     {
       // There is no need to emit line number for an unconditional branch.

clang/lib/CodeGen/CodeGenFunction.h

bool ConstantFoldsToSimpleInteger(const Expr *Cond, llvm::APSInt &Result,
                                     bool AllowLabels = false);
 
   /// EmitBranchOnBoolExpr - Emit a branch on a boolean condition (e.g. for an
   /// if statement) to the specified blocks.  Based on the condition, this might
   /// try to simplify the codegen of the conditional based on the branch.
   /// TrueCount should be the number of times we expect the condition to
   /// evaluate to true based on PGO data.
   void EmitBranchOnBoolExpr(const Expr *Cond, llvm::BasicBlock *TrueBlock,
-                            llvm::BasicBlock *FalseBlock, uint64_t TrueCount);
+                            llvm::BasicBlock *FalseBlock, uint64_t TrueCount,
+                            llvm::MDNode *Weights = nullptr);
 
   /// Given an assignment `*LHS = RHS`, emit a test that checks if \p RHS is
   /// nonnull, if \p LHS is marked _Nonnull.
   void EmitNullabilityCheck(LValue LHS, llvm::Value *RHS, SourceLocation Loc);
 
   /// An enumeration which makes it easier to specify whether or not an
   /// operation is a subtraction.
   enum { NotSubtraction = false, IsSubtraction = true };

clang/lib/CodeGen/CodeGenFunction.cpp

bool CodeGenFunction::ConstantFoldsToSimpleInteger(const Expr *Cond,
   llvm::APSInt Int = Result.Val.getInt();
   if (!AllowLabels && CodeGenFunction::ContainsLabel(Cond))
     return false;  // Contains a label.
 
   ResultInt = Int;
   return true;
 }
 
-
-
 /// EmitBranchOnBoolExpr - Emit a branch on a boolean condition (e.g. for an if
 /// statement) to the specified blocks.  Based on the condition, this might try
 /// to simplify the codegen of the conditional based on the branch.
-///
+/// \param Weights The weights determined by the likelihood attributes.
 void CodeGenFunction::EmitBranchOnBoolExpr(const Expr *Cond,
                                            llvm::BasicBlock *TrueBlock,
                                            llvm::BasicBlock *FalseBlock,
-                                           uint64_t TrueCount) {
+                                           uint64_t TrueCount,
+                                           llvm::MDNode *Weights) {
   Cond = Cond->IgnoreParens();
 
   if (const BinaryOperator *CondBOp = dyn_cast<BinaryOperator>(Cond)) {
 
     // Handle X && Y in a condition.
     if (CondBOp->getOpcode() == BO_LAnd) {
       // If we have "1 && X", simplify the code.  "0 && X" would have constant
       // folded if the case was simple enough.
       bool ConstantBool = false;
       if (ConstantFoldsToSimpleInteger(CondBOp->getLHS(), ConstantBool) &&
           ConstantBool) {
         // br(1 && X) -> br(X).
         incrementProfileCounter(CondBOp);
         return EmitBranchOnBoolExpr(CondBOp->getRHS(), TrueBlock, FalseBlock,
-                                    TrueCount);
+                                    TrueCount, Weights);
       }
 
       // If we have "X && 1", simplify the code to use an uncond branch.
       // "X && 0" would have been constant folded to 0.
       if (ConstantFoldsToSimpleInteger(CondBOp->getRHS(), ConstantBool) &&
           ConstantBool) {
         // br(X && 1) -> br(X).
         return EmitBranchOnBoolExpr(CondBOp->getLHS(), TrueBlock, FalseBlock,
-                                    TrueCount);
+                                    TrueCount, Weights);
       }
 
       // Emit the LHS as a conditional.  If the LHS conditional is false, we
       // want to jump to the FalseBlock.
       llvm::BasicBlock *LHSTrue = createBasicBlock("land.lhs.true");
       // The counter tells us how often we evaluate RHS, and all of TrueCount
       // can be propagated to that branch.
       uint64_t RHSCount = getProfileCount(CondBOp->getRHS());
 
       ConditionalEvaluation eval(*this);
       {
         ApplyDebugLocation DL(*this, Cond);
-        EmitBranchOnBoolExpr(CondBOp->getLHS(), LHSTrue, FalseBlock, RHSCount);
+        EmitBranchOnBoolExpr(CondBOp->getLHS(), LHSTrue, FalseBlock, RHSCount,
+                             Weights);
         EmitBlock(LHSTrue);
       }
 
       incrementProfileCounter(CondBOp);
       setCurrentProfileCount(getProfileCount(CondBOp->getRHS()));
 
       // Any temporaries created here are conditional.
       eval.begin(*this);
-      EmitBranchOnBoolExpr(CondBOp->getRHS(), TrueBlock, FalseBlock, TrueCount);
+      EmitBranchOnBoolExpr(CondBOp->getRHS(), TrueBlock, FalseBlock, TrueCount,
+                           Weights);
       eval.end(*this);
 
       return;
     }
 
     if (CondBOp->getOpcode() == BO_LOr) {
       // If we have "0 || X", simplify the code.  "1 || X" would have constant
       // folded if the case was simple enough.
       bool ConstantBool = false;
       if (ConstantFoldsToSimpleInteger(CondBOp->getLHS(), ConstantBool) &&
           !ConstantBool) {
         // br(0 || X) -> br(X).
         incrementProfileCounter(CondBOp);
         return EmitBranchOnBoolExpr(CondBOp->getRHS(), TrueBlock, FalseBlock,
-                                    TrueCount);
+                                    TrueCount, Weights);
       }
 
       // If we have "X || 0", simplify the code to use an uncond branch.
       // "X || 1" would have been constant folded to 1.
       if (ConstantFoldsToSimpleInteger(CondBOp->getRHS(), ConstantBool) &&
           !ConstantBool) {
         // br(X || 0) -> br(X).
         return EmitBranchOnBoolExpr(CondBOp->getLHS(), TrueBlock, FalseBlock,
-                                    TrueCount);
+                                    TrueCount, Weights);
       }
 
       // Emit the LHS as a conditional.  If the LHS conditional is true, we
       // want to jump to the TrueBlock.
       llvm::BasicBlock *LHSFalse = createBasicBlock("lor.lhs.false");
       // We have the count for entry to the RHS and for the whole expression
       // being true, so we can divy up True count between the short circuit and
       // the RHS.
       uint64_t LHSCount =
           getCurrentProfileCount() - getProfileCount(CondBOp->getRHS());
       uint64_t RHSCount = TrueCount - LHSCount;
 
       ConditionalEvaluation eval(*this);
       {
         ApplyDebugLocation DL(*this, Cond);
-        EmitBranchOnBoolExpr(CondBOp->getLHS(), TrueBlock, LHSFalse, LHSCount);
+        EmitBranchOnBoolExpr(CondBOp->getLHS(), TrueBlock, LHSFalse, LHSCount,
+                             Weights);
         EmitBlock(LHSFalse);
       }
 
       incrementProfileCounter(CondBOp);
       setCurrentProfileCount(getProfileCount(CondBOp->getRHS()));
 
       // Any temporaries created here are conditional.
       eval.begin(*this);
-      EmitBranchOnBoolExpr(CondBOp->getRHS(), TrueBlock, FalseBlock, RHSCount);
+      EmitBranchOnBoolExpr(CondBOp->getRHS(), TrueBlock, FalseBlock, RHSCount,
+                           Weights);
 
       eval.end(*this);
 
       return;
     }
   }
 
   if (const UnaryOperator *CondUOp = dyn_cast<UnaryOperator>(Cond)) {
     // br(!x, t, f) -> br(x, f, t)
     if (CondUOp->getOpcode() == UO_LNot) {
       // Negate the count.
       uint64_t FalseCount = getCurrentProfileCount() - TrueCount;
       // Negate the condition and swap the destination blocks.
       return EmitBranchOnBoolExpr(CondUOp->getSubExpr(), FalseBlock, TrueBlock,
-                                  FalseCount);
+                                  FalseCount, Weights);
     }
   }
 
   if (const ConditionalOperator *CondOp = dyn_cast<ConditionalOperator>(Cond)) {
     // br(c ? x : y, t, f) -> br(c, br(x, t, f), br(y, t, f))
     llvm::BasicBlock *LHSBlock = createBasicBlock("cond.true");
     llvm::BasicBlock *RHSBlock = createBasicBlock("cond.false");
 
     ConditionalEvaluation cond(*this);
     EmitBranchOnBoolExpr(CondOp->getCond(), LHSBlock, RHSBlock,
-                         getProfileCount(CondOp));
+                         getProfileCount(CondOp), Weights);
 
     // When computing PGO branch weights, we only know the overall count for
     // the true block. This code is essentially doing tail duplication of the
     // naive code-gen, introducing new edges for which counts are not
     // available. Divide the counts proportionally between the LHS and RHS of
     // the conditional operator.
     uint64_t LHSScaledTrueCount = 0;
     if (TrueCount) {
       double LHSRatio =
           getProfileCount(CondOp) / (double)getCurrentProfileCount();
       LHSScaledTrueCount = TrueCount * LHSRatio;
     }
 
     cond.begin(*this);
     EmitBlock(LHSBlock);
     incrementProfileCounter(CondOp);
     {
       ApplyDebugLocation DL(*this, Cond);
       EmitBranchOnBoolExpr(CondOp->getLHS(), TrueBlock, FalseBlock,
-                           LHSScaledTrueCount);
+                           LHSScaledTrueCount, Weights);
     }
     cond.end(*this);
 
     cond.begin(*this);
     EmitBlock(RHSBlock);
     EmitBranchOnBoolExpr(CondOp->getRHS(), TrueBlock, FalseBlock,
-                         TrueCount - LHSScaledTrueCount);
+                         TrueCount - LHSScaledTrueCount, Weights);
     cond.end(*this);
 
     return;
   }
 
   if (const CXXThrowExpr *Throw = dyn_cast<CXXThrowExpr>(Cond)) {
     // Conditional operator handling can give us a throw expression as a
     // condition for a case like:
if (Call && CGM.getCodeGenOpts().OptimizationLevel != 0) {
     if (FD && FD->getBuiltinID() == Builtin::BI__builtin_unpredictable) {
       llvm::MDBuilder MDHelper(getLLVMContext());
       Unpredictable = MDHelper.createUnpredictable();
     }
   }
 
   // Create branch weights based on the number of times we get here and the
   // number of times the condition should be true.
+  if (!Weights) {
     uint64_t CurrentCount = std::max(getCurrentProfileCount(), TrueCount);
-  llvm::MDNode *Weights =
-      createProfileWeights(TrueCount, CurrentCount - TrueCount);
+    Weights = createProfileWeights(TrueCount, CurrentCount - TrueCount);
+  }
 
   // Emit the code with the fully general case.
   llvm::Value *CondV;
   {
     ApplyDebugLocation DL(*this, Cond);
     CondV = EvaluateExprAsBool(Cond);
   }
   Builder.CreateCondBr(CondV, TrueBlock, FalseBlock, Weights, Unpredictable);

clang/lib/Parse/ParseDeclCXX.cpp

static bool IsBuiltInOrStandardCXX11Attribute(IdentifierInfo *AttrName,
                                               IdentifierInfo *ScopeName) {
   switch (
       ParsedAttr::getParsedKind(AttrName, ScopeName, ParsedAttr::AS_CXX11)) {
   case ParsedAttr::AT_CarriesDependency:
   case ParsedAttr::AT_Deprecated:
   case ParsedAttr::AT_FallThrough:
   case ParsedAttr::AT_CXX11NoReturn:
   case ParsedAttr::AT_NoUniqueAddress:
+  case ParsedAttr::AT_Likely:
+  case ParsedAttr::AT_Unlikely:
     return true;
   case ParsedAttr::AT_WarnUnusedResult:
     return !ScopeName && AttrName->getName().equals("nodiscard");
   case ParsedAttr::AT_Unused:
     return !ScopeName && AttrName->getName().equals("maybe_unused");
   default:
     return false;
   }

clang/lib/Sema/SemaStmt.cpp

void VisitBinaryOperator(BinaryOperator *E) {
     if (E->getOpcode() == BO_Comma)
       SemaRef.DiagnoseCommaOperator(E->getLHS(), E->getExprLoc());
     EvaluatedExprVisitor<CommaVisitor>::VisitBinaryOperator(E);
   }
 };
 }
 
 StmtResult Sema::ActOnIfStmt(SourceLocation IfLoc, bool IsConstexpr,
                              SourceLocation LParenLoc, Stmt *InitStmt,
                              ConditionResult Cond, SourceLocation RParenLoc,
                              Stmt *thenStmt, SourceLocation ElseLoc,
                              Stmt *elseStmt) {
   if (Cond.isInvalid())
     Cond = ConditionResult(
         *this, nullptr,
         MakeFullExpr(new (Context) OpaqueValueExpr(SourceLocation(),
                                                    Context.BoolTy, VK_RValue),
                      IfLoc),
         false);
 
   Expr *CondExpr = Cond.get().second;
   // Only call the CommaVisitor when not C89 due to differences in scope flags.
   if ((getLangOpts().C99 || getLangOpts().CPlusPlus) &&
       !Diags.isIgnored(diag::warn_comma_operator, CondExpr->getExprLoc()))
     CommaVisitor(*this).Visit(CondExpr);
 
   if (!elseStmt)
     DiagnoseEmptyStmtBody(CondExpr->getEndLoc(), thenStmt,
                           diag::warn_empty_if_body);
 
+  std::tuple<bool, const Attr *, const Attr *> LHC =
+      Stmt::determineLikelihoodConflict(thenStmt, elseStmt);
+  if (std::get<0>(LHC)) {
+    const Attr *ThenAttr = std::get<1>(LHC);
+    const Attr *ElseAttr = std::get<2>(LHC);
+    Diags.Report(ThenAttr->getLocation(),
+                 diag::warn_attributes_likelihood_ifstmt_conflict)
+        << ThenAttr << ThenAttr->getRange();
+    Diags.Report(ElseAttr->getLocation(), diag::note_conflicting_attribute)
+        << ElseAttr << ElseAttr->getRange();
+  }
+
   return BuildIfStmt(IfLoc, IsConstexpr, LParenLoc, InitStmt, Cond, RParenLoc,
                      thenStmt, ElseLoc, elseStmt);
 }
 
 StmtResult Sema::BuildIfStmt(SourceLocation IfLoc, bool IsConstexpr,
                              SourceLocation LParenLoc, Stmt *InitStmt,
                              ConditionResult Cond, SourceLocation RParenLoc,
                              Stmt *thenStmt, SourceLocation ElseLoc,
                              Stmt *elseStmt) {
   if (Cond.isInvalid())
     return StmtError();
 
   if (IsConstexpr || isa<ObjCAvailabilityCheckExpr>(Cond.get().second))
     setFunctionHasBranchProtectedScope();
 

clang/lib/Sema/SemaStmtAttr.cpp

if (!CEF.foundCallExpr()) {
     S.Diag(St->getBeginLoc(), diag::warn_nomerge_attribute_ignored_in_stmt)
         << NMA.getSpelling();
     return nullptr;
   }
 
   return ::new (S.Context) NoMergeAttr(S.Context, A);
 }
 
+static Attr *handleLikely(Sema &S, Stmt *St, const ParsedAttr &A,
+                          SourceRange Range) {
+
+  if (!S.getLangOpts().CPlusPlus20 && A.isCXX11Attribute() && !A.getScopeName())
+    S.Diag(A.getLoc(), diag::ext_cxx20_attr) << A << Range;
+
+  return ::new (S.Context) LikelyAttr(S.Context, A);
+}
+
+static Attr *handleUnlikely(Sema &S, Stmt *St, const ParsedAttr &A,
+                            SourceRange Range) {
+
+  if (!S.getLangOpts().CPlusPlus20 && A.isCXX11Attribute() && !A.getScopeName())
+    S.Diag(A.getLoc(), diag::ext_cxx20_attr) << A << Range;
+
+  return ::new (S.Context) UnlikelyAttr(S.Context, A);
+}
+
 static void
 CheckForIncompatibleAttributes(Sema &S,
                                const SmallVectorImpl<const Attr *> &Attrs) {
   // There are 6 categories of loop hints attributes: vectorize, interleave,
   // unroll, unroll_and_jam, pipeline and distribute. Except for distribute they
   // come in two variants: a state form and a numeric form.  The state form
   // selectively defaults/enables/disables the transformation for the loop
   // (for unroll, default indicates full unrolling rather than enabling the
if (CategoryState.StateAttr && CategoryState.NumericAttr &&
       // compatible with enable or full form of the unroll pragma because these
       // directives indicate full unrolling.
       S.Diag(OptionLoc, diag::err_pragma_loop_compatibility)
           << /*Duplicate=*/false
           << CategoryState.StateAttr->getDiagnosticName(Policy)
           << CategoryState.NumericAttr->getDiagnosticName(Policy);
     }
   }
+
+  // C++20 [dcl.attr.likelihood]p1 The attribute-token likely shall not appear
+  // in an attribute-specifier-seq that contains the attribute-token unlikely.
+  const LikelyAttr *Likely = nullptr;
+  const UnlikelyAttr *Unlikely = nullptr;
+  for (const auto *I : Attrs) {
+    if (const auto *Attr = dyn_cast<LikelyAttr>(I)) {
+      if (Unlikely) {
+        S.Diag(Attr->getLocation(), diag::err_attributes_are_not_compatible)
+            << Attr << Unlikely << Attr->getRange();
+        S.Diag(Unlikely->getLocation(), diag::note_conflicting_attribute)
+            << Unlikely->getRange();
+        return;
+      }
+      Likely = Attr;
+    } else if (const auto *Attr = dyn_cast<UnlikelyAttr>(I)) {
+      if (Likely) {
+        S.Diag(Attr->getLocation(), diag::err_attributes_are_not_compatible)
+            << Attr << Likely << Attr->getRange();
+        S.Diag(Likely->getLocation(), diag::note_conflicting_attribute)
+            << Likely->getRange();
+        return;
+      }
+      Unlikely = Attr;
+    }
+  }
 }
 
 static Attr *handleOpenCLUnrollHint(Sema &S, Stmt *St, const ParsedAttr &A,
                                     SourceRange Range) {
   // Although the feature was introduced only in OpenCL C v2.0 s6.11.5, it's
   // useful for OpenCL 1.x too and doesn't require HW support.
   // opencl_unroll_hint can have 0 arguments (compiler
   // determines unrolling factor) or 1 argument (the unroll factor provided
static Attr *ProcessStmtAttribute(Sema &S, Stmt *St, const ParsedAttr &A,
   case ParsedAttr::AT_LoopHint:
     return handleLoopHintAttr(S, St, A, Range);
   case ParsedAttr::AT_OpenCLUnrollHint:
     return handleOpenCLUnrollHint(S, St, A, Range);
   case ParsedAttr::AT_Suppress:
     return handleSuppressAttr(S, St, A, Range);
   case ParsedAttr::AT_NoMerge:
     return handleNoMergeAttr(S, St, A, Range);
+  case ParsedAttr::AT_Likely:
+    return handleLikely(S, St, A, Range);
+  case ParsedAttr::AT_Unlikely:
+    return handleUnlikely(S, St, A, Range);
   default:
     // if we're here, then we parsed a known attribute, but didn't recognize
     // it as a statement attribute => it is declaration attribute
     S.Diag(A.getRange().getBegin(), diag::err_decl_attribute_invalid_on_stmt)
         << A << St->getBeginLoc();
     return nullptr;
   }
 }

clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp

+// RUN: %clang_cc1 -O1 -emit-llvm %s -o - -triple=x86_64-linux-gnu | FileCheck -DLIKELY=2000 -DUNLIKELY=1 %s
+// RUN: %clang_cc1 -O1 -emit-llvm %s -triple=x86_64-linux-gnu -mllvm -likely-branch-weight=99 -mllvm -unlikely-branch-weight=42 -o - | FileCheck -DLIKELY=99 -DUNLIKELY=42 %s
+
+extern volatile bool b;
+extern volatile int i;
+extern bool A();
+extern bool B();
+
+bool f() {
+  // CHECK-LABEL: define zeroext i1 @_Z1fv
+  // CHECK: br {{.*}} !prof !7
+  if (b)
+    [[likely]] {
+      return A();
+    }
+  return B();
+}
+
+bool g() {
+  // CHECK-LABEL: define zeroext i1 @_Z1gv
+  // CHECK: br {{.*}} !prof !8
+  if (b)
+    [[unlikely]] {
+      return A();
+    }
+
+  return B();
+}
+
+bool h() {
+  // CHECK-LABEL: define zeroext i1 @_Z1hv
+  // CHECK: br {{.*}} !prof !8
+  if (b)
+    [[unlikely]] return A();
+
+  return B();
+}
+
+void NullStmt() {
+  // CHECK-LABEL: define{{.*}}NullStmt
+  // CHECK: br {{.*}} !prof !8
+  if (b)
+    [[unlikely]];
+  else {
+    // Make sure the branches aren't optimized away.
+    b = true;
+  }
+}
+
+void IfStmt() {
+  // CHECK-LABEL: define{{.*}}IfStmt
+  // CHECK: br {{.*}} !prof !8
+  if (b)
+    [[unlikely]] if (B()) {}
+
+  // CHECK-NOT: br {{.*}} !prof
+  // CHECK: br {{.*}} !prof
+  if (b) {
+    if (B())
+      [[unlikely]] { b = false; }
+  }
+}
+
+void WhileStmt() {
+  // CHECK-LABEL: define{{.*}}WhileStmt
+  // CHECK: br {{.*}} !prof !8
+  if (b)
+    [[unlikely]] while (B()) {}
+
+  // CHECK-NOT: br {{.*}} %if.end{{.*}} !prof
+  if (b)
+    while (B())
+      [[unlikely]] { b = false; }
+}
+
+void DoStmt() {
+  // CHECK-LABEL: define{{.*}}DoStmt
+  // CHECK: br {{.*}} !prof !8
+  if (b)
+    [[unlikely]] do {}
+    while (B())
+      ;
+
+  // CHECK-NOT: br {{.*}} %if.end{{.*}} !prof
+  if (b)
+    do
+      [[unlikely]] {}
+    while (B());
+}
+
+void ForStmt() {
+  // CHECK-LABEL: define{{.*}}ForStmt
+  // CHECK: br {{.*}} !prof !8
+  if (b)
+    [[unlikely]] for (; B();) {}
+
+  // CHECK-NOT: br {{.*}} %if.end{{.*}} !prof
+  if (b)
+    for (; B();)
+      [[unlikely]] {}
+}
+
+void GotoStmt() {
+  // CHECK-LABEL: define{{.*}}GotoStmt
+  // CHECK: br {{.*}} !prof !8
+  if (b)
+    [[unlikely]] goto end;
+  else {
+    // Make sure the branches aren't optimized away.
+    b = true;
+  }
+end:;
+}
+
+void ReturnStmt() {
+  // CHECK-LABEL: define{{.*}}ReturnStmt
+  // CHECK: br {{.*}} !prof !8
+  if (b)
+    [[unlikely]] return;
+  else {
+    // Make sure the branches aren't optimized away.
+    b = true;
+  }
+}
+
+void SwitchStmt() {
+  // CHECK-LABEL: define{{.*}}SwitchStmt
+  // CHECK: br {{.*}} !prof !8
+  if (b)
+    [[unlikely]] switch (i) {}
+  else {
+    // Make sure the branches aren't optimized away.
+    b = true;
+  }
+  // CHECK-NOT: br {{.*}} %if.end{{.*}} !prof
+  if (b)
+    switch (i)
+      [[unlikely]] {}
+  else {
+    // Make sure the branches aren't optimized away.
+    b = true;
+  }
+}
+
+// CHECK: !7 = !{!"branch_weights", i32 [[UNLIKELY]], i32 [[LIKELY]]}
+// CHECK: !8 = !{!"branch_weights", i32 [[LIKELY]], i32 [[UNLIKELY]]}

clang/test/Preprocessor/has_attribute.cpp

 CXX11(noreturn)
 CXX11(unlikely)
 // FIXME(201806L) CHECK: assert: 0
 // CHECK: carries_dependency: 200809L
 // CHECK: deprecated: 201309L
 // FIXME(201806L) CHECK: ensures: 0
 // FIXME(201806L) CHECK: expects: 0
 // CHECK: fallthrough: 201603L
-// FIXME(201803L) CHECK: likely: 0
+// FIXME(201803L) CHECK: likely: 2L
 // CHECK: maybe_unused: 201603L
 // ITANIUM: no_unique_address: 201803L
 // WINDOWS: no_unique_address: 0
 // CHECK: nodiscard: 201907L
 // CHECK: noreturn: 200809L
-// FIXME(201803L) CHECK: unlikely: 0
+// FIXME(201803L) CHECK: unlikely: 2L
 
 // Test for Microsoft __declspec attributes
 
 #define DECLSPEC(x) x: __has_declspec_attribute(x)
 
 // CHECK: uuid: 1
 // CHECK: __uuid__: 1
 DECLSPEC(uuid)
 DECLSPEC(__uuid__)
 
 // CHECK: fallthrough: 0
 DECLSPEC(fallthrough)

clang/test/Sema/attr-likelihood.c

+// RUN: %clang_cc1 %s -fsyntax-only -fdouble-square-bracket-attributes -verify
+
+void g() {
+  if (1)
+    [[clang::likely]] {}
+}
+void m() {
+  [[clang::likely]] int x = 42; // expected-error {{'likely' attribute cannot be applied to a declaration}}
+
+  if (x)
+    [[clang::unlikely]] {}
+  if (x) {
+    [[clang::unlikely]];
+  }
+  switch (x) {
+  case 1:
+    [[clang::likely]] {}
+    break;
+    [[clang::likely]] case 2 : case 3 : {}
+    break;
+  }
+
+  do {
+    [[clang::unlikely]];
+  } while (x);
+  do
+    [[clang::unlikely]] {}
+  while (x);
+  do { // expected-note {{to match this 'do'}}
+  }
+  [[clang::unlikely]] while (x); // expected-error {{expected 'while' in do/while loop}}
+  for (;;)
+    [[clang::unlikely]] {}
+  for (;;) {
+    [[clang::unlikely]];
+  }
+  while (x)
+    [[clang::unlikely]] {}
+  while (x) {
+    [[clang::unlikely]];
+  }
+
+  if (x)
+    goto lbl;
+
+  // FIXME: allow the attribute on the label
+  [[clang::unlikely]] lbl : // expected-error {{'unlikely' attribute cannot be applied to a declaration}}
+  [[clang::likely]] x = x + 1;
+
+  [[clang::likely]]++ x;
+}

clang/test/SemaCXX/attr-likelihood.cpp

+// RUN: %clang_cc1 %s -fsyntax-only -verify
+// RUN: %clang_cc1 %s -DPEDANTIC -pedantic -fsyntax-only -verify
+
+#if PEDANTIC
+void g() {
+  if (true)
+    [[likely]] {} // expected-warning {{use of the 'likely' attribute is a C++20 extension}}
+  else
+    [[unlikely]] {} // expected-warning {{use of the 'unlikely' attribute is a C++20 extension}}
+}
+#else
+void a() {
+  if (true)
+    [[likely]]; // expected-warning {{conflicting attributes 'likely' are ignored}}
+  else
+    [[likely]]; // expected-note {{conflicting attribute is here}}
+}
+
+void b() {
+  if (true)
+    [[unlikely]]; // expected-warning {{conflicting attributes 'unlikely' are ignored}}
+  else
+    [[unlikely]]; // expected-note {{conflicting attribute is here}}
+}
+
+void c() {
+  if (true)
+    [[likely]];
+}
+
+void d() {
+  if (true)
+    [[unlikely]];
+}
+
+void g() {
+  if (true)
+    [[likely]] {}
+  else
+    [[unlikely]] {}
+}
+
+void h() {
+  if (true)
+    [[likely]] {}
+  else {
+  }
+}
+
+void i() {
+  if (true)
+    [[unlikely]] {}
+  else {
+  }
+}
+
+void j() {
+  if (true) {
+  } else
+    [[likely]] {}
+}
+
+void k() {
+  if (true) {
+  } else
+    [[likely]] {}
+}
+
+void l() {
+  if (true)
+    [[likely]] {}
+  else
+    [[unlikely]] if (false) [[likely]] {}
+}
+
+void m() {
+  [[likely]] int x = 42; // expected-error {{'likely' attribute cannot be applied to a declaration}}
+
+  if (x)
+    [[unlikely]] {}
+  if (x) {
+    [[unlikely]];
+  }
+  switch (x) {
+  case 1:
+    [[likely]] {}
+    break;
+    [[likely]] case 2 : case 3 : {}
+    break;
+  }
+
+  do {
+    [[unlikely]];
+  } while (x);
+  do
+    [[unlikely]] {}
+  while (x);
+  do { // expected-note {{to match this 'do'}}
+  }
+  [[unlikely]] while (x); // expected-error {{expected 'while' in do/while loop}}
+  for (;;)
+    [[unlikely]] {}
+  for (;;) {
+    [[unlikely]];
+  }
+  while (x)
+    [[unlikely]] {}
+  while (x) {
+    [[unlikely]];
+  }
+
+  switch (x)
+    [[unlikely]] {}
+
+  if (x)
+    goto lbl;
+
+  // FIXME: allow the attribute on the label
+  [[unlikely]] lbl : // expected-error {{'unlikely' attribute cannot be applied to a declaration}}
+                     [[likely]] x = x + 1;
+
+  [[likely]]++ x;
+}
+
+void n() [[likely]] // expected-error {{'likely' attribute cannot be applied to types}}
+{
+  try
+    [[likely]] {} // expected-error {{expected '{'}}
+  catch (...) [[likely]] { // expected-error {{expected expression}}
+  }
+}
+#endif

clang/www/cxx_status.html

</tr>
     <tr>
       <td><tt>[[no_unique_address]]</tt> attribute</td>
       <td><a href="https://wg21.link/p0840r2">P0840R2</a></td>
       <td class="full" align="center">Clang 9</td>
     </tr>
     <tr>
       <td><tt>[[likely]]</tt> and <tt>[[unlikely]]</tt> attributes</td>
       <td><a href="https://wg21.link/p0479r5">P0479R5</a></td>
-      <td class="none" align="center">No</td>
+      <td class="none" align="center">Clang 12 (partial)</td>
     </tr>
     <tr>
       <td><tt>typename</tt> optional in more contexts</td>
       <td><a href="https://wg21.link/p0634r3">P0634R3</a></td>
       <td class="none" align="center">No</td>
     </tr>
     <tr>
       <td>Pack expansion in lambda <i>init-capture</i></td>

llvm/include/llvm/Transforms/Scalar/LowerExpectIntrinsic.h

 ///
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_TRANSFORMS_SCALAR_LOWEREXPECTINTRINSIC_H
 #define LLVM_TRANSFORMS_SCALAR_LOWEREXPECTINTRINSIC_H
 
 #include "llvm/IR/Function.h"
 #include "llvm/IR/PassManager.h"
+#include "llvm/Support/CommandLine.h"
 
 namespace llvm {
 
 struct LowerExpectIntrinsicPass : PassInfoMixin<LowerExpectIntrinsicPass> {
   /// Run the pass over the function.
   ///
   /// This will lower all of the expect intrinsic calls in this function into
   /// branch weight metadata. That metadata will subsequently feed the analysis
   /// of the probabilities and frequencies of the CFG. After running this pass,
   /// no more expect intrinsics remain, allowing the rest of the optimizer to
   /// ignore them.
   PreservedAnalyses run(Function &F, FunctionAnalysisManager &);
 };
 
+extern cl::opt<uint32_t> LikelyBranchWeight;
+extern cl::opt<uint32_t> UnlikelyBranchWeight;
 }
 
 #endif

llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp

 #include "llvm/IR/Function.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Pass.h"
-#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Transforms/Scalar.h"
 #include "llvm/Transforms/Utils/MisExpect.h"
 
 using namespace llvm;
 
 #define DEBUG_TYPE "lower-expect-intrinsic"
 
 STATISTIC(ExpectIntrinsicsHandled,
           "Number of 'expect' intrinsic instructions handled");
 
 // These default values are chosen to represent an extremely skewed outcome for
 // a condition, but they leave some room for interpretation by later passes.
 //
 // If the documentation for __builtin_expect() was made explicit that it should
 // only be used in extreme cases, we could make this ratio higher. As it stands,
 // programmers may be using __builtin_expect() / llvm.expect to annotate that a
 // branch is likely or unlikely to be taken.
 //
 // There is a known dependency on this ratio in CodeGenPrepare when transforming
 // 'select' instructions. It may be worthwhile to hoist these values to some
 // shared space, so they can be used directly by other passes.
 
-static cl::opt<uint32_t> LikelyBranchWeight(
+cl::opt<uint32_t> llvm::LikelyBranchWeight(
     "likely-branch-weight", cl::Hidden, cl::init(2000),
     cl::desc("Weight of the branch likely to be taken (default = 2000)"));
-static cl::opt<uint32_t> UnlikelyBranchWeight(
+cl::opt<uint32_t> llvm::UnlikelyBranchWeight(
     "unlikely-branch-weight", cl::Hidden, cl::init(1),
     cl::desc("Weight of the branch unlikely to be taken (default = 1)"));
 
 static std::tuple<uint32_t, uint32_t>
 getBranchWeight(Intrinsic::ID IntrinsicID, CallInst *CI, int BranchCount) {
   if (IntrinsicID == Intrinsic::expect) {
     // __builtin_expect
     return std::make_tuple(LikelyBranchWeight.getValue(),