unlikely intrinsic does not work

[stepancheg]

Code:

#![feature(core_intrinsics)]

use std::intrinsics::unlikely;

extern "C" {
    fn aaa() -> u32;
    fn bbb() -> u32;
}

pub unsafe fn foo(a: bool) -> u32 {
    if unlikely(a) { aaa() } else { bbb() }
}

Compiler explorer

rustc 1.57.0-nightly (fdf6505 2021-09-07)

Generates assembly:

example::foo:
        test    edi, edi
        je      .LBB0_1
        jmp     qword ptr [rip + aaa@GOTPCREL]
.LBB0_1:
        jmp     qword ptr [rip + bbb@GOTPCREL]

If unlikely worked, call to aaa would be after jump (it works if aaa is marked #[cold] for example).

Unlikely is present in Rust MIR:

fn foo(_1: bool) -> u32 {
    debug a => _1;                       // in scope 0 at /app/example.rs:10:19: 10:20
    let mut _0: u32;                     // return place in scope 0 at /app/example.rs:10:31: 10:34
    let mut _2: bool;                    // in scope 0 at /app/example.rs:11:8: 11:19
    let mut _3: bool;                    // in scope 0 at /app/example.rs:11:17: 11:18

    bb0: {
        _3 = _1;                         // scope 0 at /app/example.rs:11:17: 11:18
        _2 = unlikely(move _3) -> bb1;   // scope 0 at /app/example.rs:11:8: 11:19
                                         // mir::Constant
                                         // + span: /app/example.rs:11:8: 11:16
                                         // + literal: Const { ty: extern "rust-intrinsic" fn(bool) -> bool {std::intrinsics::unlikely}, val: Value(Scalar(<ZST>)) }
    }

    bb1: {
        switchInt(move _2) -> [false: bb3, otherwise: bb2]; // scope 0 at /app/example.rs:11:8: 11:19
    }

    bb2: {
        _0 = aaa() -> bb4;               // scope 0 at /app/example.rs:11:22: 11:27
                                         // mir::Constant
                                         // + span: /app/example.rs:11:22: 11:25
                                         // + literal: Const { ty: unsafe extern "C" fn() -> u32 {aaa}, val: Value(Scalar(<ZST>)) }
    }

    bb3: {
        _0 = bbb() -> bb4;               // scope 0 at /app/example.rs:11:37: 11:42
                                         // mir::Constant
                                         // + span: /app/example.rs:11:37: 11:40
                                         // + literal: Const { ty: unsafe extern "C" fn() -> u32 {bbb}, val: Value(Scalar(<ZST>)) }
    }

    bb4: {
        return;                          // scope 0 at /app/example.rs:12:2: 12:2
    }
}

but it is lost in LLVM IR:

define i32 @_ZN7example3foo17hdda9a45d5ad75400E(i1 zeroext %a) unnamed_addr #0 !dbg !6 {
start:
  br i1 %a, label %bb2, label %bb3, !dbg !10

bb3:                                              ; preds = %start
  %0 = tail call i32 @bbb(), !dbg !11
  br label %bb4, !dbg !11

bb2:                                              ; preds = %start
  %1 = tail call i32 @aaa(), !dbg !12
  br label %bb4, !dbg !12

bb4:                                              ; preds = %bb3, %bb2
  %.0 = phi i32 [ %1, %bb2 ], [ %0, %bb3 ], !dbg !13
  ret i32 %.0, !dbg !14
}

[nagisa]

Rust generates the call to expect so what's happening here is that some LLVM optimization pass simply considers it to be profitable to remove the call.

[nikic]

This probably got broken by https://reviews.llvm.org/D100213. Expect intrinsics are now lowered before SROA.

[nikic]

The probably most reliable way to fix this would be to not use llvm.expect at all, and directly emit !prof metadata in rustc.

[Kobzol]

Just to be clear, you mean this? So instead of using llvm.expect on the if condition value, we would generate the branch weight metadata directly on the br instruction, do I understand it correctly?

[nikic]

Yes, that's right. This is something we'd definitely need for the more elaborate attribute-based approached discussed in #26179, so it would probably make sense to switch to this approach for the intrinsic as well.

[RalfJung]

This doesn't really work for the intrinsic, since the intrinsic can be used anywhere, not just in an if.

It would be somewhat disconcerning if the intrinsic somehow "pattern-matched on the surrounding code" to do its thing.

[Kobzol]

I think that the plan in #26179 is to migrate to an attribute (e.g. #[likely]), which would mostly be used on conditions and stuff. But I also suppose that we can keep both the likely/unlikely functions and also generate the metadata on branches if likely/unlikely is used directly inside an if (for example).

[nikic]

@RalfJung The intrinsic already only works when used directly in an if (well, I should say "used to work", currently it just doesn't work at all). llvm.expect is lowered and discarded during very early optimization, so it doesn't support any non-trivial patterns (like using it inside a function that gets inlined etc).

[nikic]

Basically, this intrinsic is already "pattern-matched on the surrounding code". But this pattern matching currently happens in LLVM, and is geared towards the IR emitted by Clang rather than rustc. Moving that pattern matching into rustc would make it more robust/predictable.

[RalfJung]

Fair. In the end it's semantically a no-op so I don't care quite as much, but attributes seem like the much better design for this sort of thing so I am glad to hear that that's what we are moving towards. :)

[SUPERCILEX]

llvm.expect is lowered and discarded during very early optimization, so it doesn't support any non-trivial patterns (like using it inside a function that gets inlined etc).

Is that an llvm bug? I guess I'm trying to understand how this broke and what the next steps are to fix the intrinsics. This bug and the new attributes should be considered separately.

[MSxDOS]

Any progress ?

[Amanieu]

On nightly this now works correctly. You can verify this by checking the example in the top comment on godbolt: switching between likely and unlikely with invert the order of the calls.