`[[likely]]` and `[[unlikely]]` ignored with jump to tail call

[eisenwave]

https://godbolt.org/z/TY4ovb9ss

void t(), f();

void decide(bool ok) {
    if (ok) [[likely]] {
        t();
    } else {
        f();
    }
}

The use of [[likely]] or [[unlikely]] here has no effect whatsoever, and regardless which attribute is applied, Clang always optimizes to:

decide(bool):
        test    edi, edi
        je      f()@PLT
        jmp     t()@PLT

When [[likely]] is applied to calling t(), this is bad output because it would be better to let je t() succeed instead of letting je f() fail and then doing jmp t(). MSVC handles attributes correctly in this case.

Initially, I thought this is some edge case and path ordering issue, but maybe it isn't. When using [[likely]], LowerExpectIntrinsicPass does:

-%loadedv.expval = call i1 @llvm.expect.i1(i1 %loadedv, i1 true)
-br i1 %loadedv.expval, label %if.then, label %if.else
+br i1 %loadedv, label %if.then, label %if.else

When replacing [[likely]] with [[unlikely]]:

-%loadedv.expval = call i1 @llvm.expect.i1(i1 %loadedv, i1 false)
-br i1 %loadedv.expval, label %if.then, label %if.else
+br i1 %loadedv, label %if.then, label %if.else

Maybe I'm not getting something about the optimization pipeline, but from that point on, the IR is identical, so it looks like [[likely]] and [[unlikely]] are always ignored completely. This also happens when the contents of if/else are something different.

[llvmbot]

@llvm/issue-subscribers-clang-codegen

Author: Jan Schultke (Eisenwave)

<https://godbolt.org/z/TY4ovb9ss> ```cpp void t(), f();

void decide(bool ok) {
if (ok) [[likely]] {
t();
} else {
f();
}
}

The use of `[[likely]]` or `[[unlikely]]` here has no effect whatsoever, and regardless which attribute is applied, Clang always optimizes to:
```cpp
decide(bool):
        test    edi, edi
        je      f()@<!-- -->PLT
        jmp     t()@<!-- -->PLT

When [[likely]] is applied to calling t(), this is bad output because it would be better to let je t() succeed instead of letting je f() fail and then doing jmp t(). MSVC handles attributes correctly in this case.

Initially, I thought this is some edge case and path ordering issue, but maybe it isn't. When using [[likely]], LowerExpectIntrinsicPass does:

-%loadedv.expval = call i1 @<!-- -->llvm.expect.i1(i1 %loadedv, i1 true)
-br i1 %loadedv.expval, label %if.then, label %if.else
+br i1 %loadedv, label %if.then, label %if.else

When replacing [[likely]] with [[unlikely]]:

-%loadedv.expval = call i1 @<!-- -->llvm.expect.i1(i1 %loadedv, i1 false)
-br i1 %loadedv.expval, label %if.then, label %if.else
+br i1 %loadedv, label %if.then, label %if.else

Maybe I'm not getting something about the optimization pipeline, but from that point on, the IR is identical, so it looks like [[likely]] and [[unlikely]] are always ignored completely. This also happens when the contents of if/else are something different.

[marcauberer]

@eisenwave the IR instructions did not change, but the attached branch_weights (!prof !28) metadata did, which tells the optimizer, which branch is more likely. Thus the output of the LowerExpectIntrinsicPass looks correct to me:

https://godbolt.org/z/bbfs6Mjvq
(CE pipeline viewer, enable "Dump Full Module" option, disable "Hide Instruction Metadata" option)

I can't tell if there is a bug in the X86 backend, as I am not familiar with it, but the Clang codegen, as well as the IR optimizer produce code, that correctly transports the likely/unlikely.

[llvmbot]

@llvm/issue-subscribers-backend-x86

Author: Jan Schultke (Eisenwave)

<https://godbolt.org/z/TY4ovb9ss> ```cpp void t(), f();

void decide(bool ok) {
if (ok) [[likely]] {
t();
} else {
f();
}
}

The use of `[[likely]]` or `[[unlikely]]` here has no effect whatsoever, and regardless which attribute is applied, Clang always optimizes to:
```cpp
decide(bool):
        test    edi, edi
        je      f()@<!-- -->PLT
        jmp     t()@<!-- -->PLT

When [[likely]] is applied to calling t(), this is bad output because it would be better to let je t() succeed instead of letting je f() fail and then doing jmp t(). MSVC handles attributes correctly in this case.

Initially, I thought this is some edge case and path ordering issue, but maybe it isn't. When using [[likely]], LowerExpectIntrinsicPass does:

-%loadedv.expval = call i1 @<!-- -->llvm.expect.i1(i1 %loadedv, i1 true)
-br i1 %loadedv.expval, label %if.then, label %if.else
+br i1 %loadedv, label %if.then, label %if.else

When replacing [[likely]] with [[unlikely]]:

-%loadedv.expval = call i1 @<!-- -->llvm.expect.i1(i1 %loadedv, i1 false)
-br i1 %loadedv.expval, label %if.then, label %if.else
+br i1 %loadedv, label %if.then, label %if.else

Maybe I'm not getting something about the optimization pipeline, but from that point on, the IR is identical, so it looks like [[likely]] and [[unlikely]] are always ignored completely. This also happens when the contents of if/else are something different.

[marcauberer]

The AArch64 backend produces assembly with the same input, that reflects the likely/unlikely:
https://godbolt.org/z/8WYGvaj69

This indicates a problem with the X86 backend.

[eisenwave]

@marcauberer I don't know much about ARM assembly, but isn't there a missed optimization here?

decide(bool):
        tbnz    w0, #0, t()
        b       f()

Seems like it would be valid output as well.

Perhaps the issue is that generally, there is an extra backend optimization for x86 which gets rid of the redundant conditional jump, but which also doesn't respect branch weights. This optimization is missing for AArch64, so the code respects branch weights but is problematic in different ways.

[marcauberer]

Maybe I am missing something, but yes, this seems to be a missed optimization. I will split this off to a separate AArch64 issue and keep this one for the x86 branch_weighting problem.

[omern1]

All other things being equal the codegen between Clang and MSVC is identical in terms of performance except for one thing, conditional branches are predicted to be not taken when there isn't a branch history for them.

Having the T call be an unconditional branch is preferable because the first time that code is seen the je will be predicted to be not taken and the jmp will be predicted to be taken, i.e. no branch misprediction (based on what we know from the likely attribute). With the code from MSVC there will be a misprediction (the jne will be predicted not taken even though based on the attribute we know that that is unlikely to be true).

I think Clang's output is actually marginally better than MSVC's.

[eisenwave]

Regardless which option is better, it's obviously a bug that [[likely]] is ignored and has no influence on the result, whether you attach or remove it.