Unfortunate optimization with Option<bool> vs Option<uint>

[alexcrichton]

Currently, given this program:

#[inline(never)]
fn get_bool(slot: &mut Option<bool>) -> &mut bool {
    if slot.is_none() {
        *slot = Some(true);
    }
    slot.as_mut().unwrap()
}

#[inline(never)]
fn get_uint(slot: &mut Option<uint>) -> &mut uint {
    if slot.is_none() {
        *slot = Some(1);
    }
    slot.as_mut().unwrap()
}

fn main() {
    println!("{}", get_bool(&mut None));
    println!("{}", get_uint(&mut None));
}

It generates this IR.

The "unfortunate" part here is that a call to failure is generated for the get_bool function where no failure is generated for the get_uint function.

There's a lot going on here so I'm not sure if it's our codegen or LLVM's codegen, but I would personally expect both cases to optimize down to never unwinding because it's known that neither Option can ever be None.

[alexcrichton]

cc @luqmana, @dotdash, @Aatch

[dotdash]

Seems to happen for all integer types smaller than int/uint

[dotdash]

Noticable difference is that the uint version does two independent stores, one for the discriminant and one for the value itself, but the versions with the smaller data types have a store for the whole Option value at once:

  store %"enum.core::option::Option<[bool]>[#3]" { i8 1, [0 x i8] undef, [1 x i8] c"\01" }, %"enum.core::option::Option<[bool]>[#3]"* %0, align 1

And I guess the pass responsible for optimizing the icmp away can't handle that.

Since we originally emit independent stores, I guess this should be handled in LLVM upstream.

[dotdash]

OTOH I can't reproduce the problem with clang. So maybe we could do something about the way unions are represented in LLVM types.

[dotdash]

OK, it's something completely different. The assignment happens via store for Option<bool> etc. but via memcpy for Option<uint>. Only the memcpy gets optimized.

[Aatch]

Because Option<bool> is only two bytes, it's an immediate. Hence why we don't use memcpy to copy it. I don't know why LLVM can optimize one and not the other though. All I can think is that the ordering of the passes is such that it combines the stores for the bool case before LLVM realises that the panic case is impossible.

[Aatch]

There is a comment in expr.rs that seems suspicious. Commenting out the line causes get_bool to optimised as expected. Since it references morestack, I'm curious if the comment is still accurate today, as it suggests that it may be related to segmented stacks.

Maybe whoever wrote the comment could chime in? The library split has made it difficult to find out who though.

[Aatch]

This commit added the comment in question: ee4ba449

@nikomatsakis can you confirm that it was related to segmented stacks?

[dotdash]

http://reviews.llvm.org/D6829 -- This teaches LLVM to handle the aggregate and extract the loaded value from it, allowing it to optimize the code even without #20161. It also handles the example I presented there where the deferred load breaks an optimization.

[alexcrichton]

Wow, thanks so much for looking into this @dotdash!