Performance of inlined code not as expected

[tbu-]

I expect the following two snippets to have the same performance:

#[inline(always)]
fn decrement(val: u32) -> Option<u32> {
    if val != 0 {
        Some(val - 1)
    } else {
        None
    }
}

match decrement(a) {
    Some(aa) => /* do something */ {},
    None => /* do something else */ {},
}

and

if a != 0 {
    a - 1;
    /* do something */
} else {
    /* do something else */
}

The latter one has better performance in current Rust: Gist.

[dotdash]

That's an interesting case, which I think LLVM should handle, but currently doesn't. Maybe the required analysis is too expensive? No idea. The problem is this:

match_case:                                       ; preds = %match_case, %match_case.lr.ph
  %.1417 = phi i32 [ %2, %match_case.lr.ph ], [ %.14, %match_case ]
; black_box omitted here
  %5 = icmp eq i32 %4, 0
  %6 = add i32 %4, -1
  %.14 = select i1 %5, i32 0, i32 %6
  br i1 %5, label %clean_ast_57_.loopexit, label %match_case

%.14 is only used in the PHI node when we are looping. And that is only the case when %5 is false, which means that %.i14 will always be equal to %6. And indeed, replacing %.14 by %6 in the PHI node leads to identical IR and ASM as the fast version.

I'll try looking around in LLVM for a bit and file a bug if I can't figure out anything.

[dotdash]

http://reviews.llvm.org/D9051

[tbu-]

@dotdash Is this already the fix?

[dotdash]

@tbu- Yes, with that patch, both examples optimize to the same IR.

Tested with:

#![crate_type="lib"]
#![feature(test)]

extern crate test;

#[inline(always)]
fn decrement(val: u32) -> Option<u32> {
    if val != 0 {
        Some(val - 1)
    } else {
        None
    }
}

pub fn testfun(mut a: u32) {
    loop {
        if a == 0 { return; }
        a = test::black_box(a - 1);
    }
}

pub fn testfun2(mut a: u32) {
    loop {
        a = test::black_box(match decrement(a) {
            Some(aa) => { aa },
            None => { return; },
        })
    }
}

[steveklabnik]

Since this is not only not our bug, but already fixed upstream, I'm going to give it a close. Thanks so much, @dotdash !

[bstrie]

Reopening for the moment, since the patch mentioned above has not yet been accepted by LLVM and may need to be backported to our fork.

[dotdash]

This has been merged upstream. Not sure if we want to close this right away or wait until we support building against an LLVM version that includes that fix.

[dotdash]

The fix landed with the LLVM update in #26025