LLVM doesn't understand Some(Box<u32>) is always some

[alexcrichton]

playground

For the following code:

use std::mem;

fn foo<T>(a: &mut T, b: T) -> bool {
    let b = Some(mem::replace(a, b));
    let ret = b.is_some();
    mem::forget(b);
    return ret
}

pub fn foo_u32(a: &mut u32, b: u32) -> bool {
    foo(a, b)
}

pub fn foo_box(a: &mut Box<u32>, b: Box<u32>) -> bool {
    foo(a, b)
}

This compiles down to the IR (optimized)

; Function Attrs: norecurse uwtable
define zeroext i1 @_ZN3foo7foo_u3217h93ddaaeb1c2cc2b2E(i32* nocapture dereferenceable(4), i32) unnamed_addr #0  {
entry-block:
  store i32 %1, i32* %0, align 4
  ret i1 true
}

; Function Attrs: norecurse uwtable
define zeroext i1 @_ZN3foo7foo_box17hd05578023690d1faE(i32** nocapture dereferenceable(8), i32* noalias dereferenceable(4)) unnamed_addr #0  {
entry-block:
  %2 = ptrtoint i32* %1 to i64
  %3 = bitcast i32** %0 to i64*
  %4 = bitcast i32** %0 to i8**
  %5 = load i8*, i8** %4, align 8, !noalias !0
  store i64 %2, i64* %3, align 8, !noalias !0
  %not.switchtmp.i.i = icmp ne i8* %5, null
  ret i1 %not.switchtmp.i.i
}

attributes #0 = { norecurse uwtable }
attributes #1 = { nounwind }

It seems odd that the Box case is not as optimized as the u32 case? This seems due to the null-pointer optimization of the Option<Box> itself. I wonder if there's more metadata we can attach to tell LLVM the loads aren't null?

[eddyb]

Probably using !nonnull on loads would work.

The fundamental problem is that it's hard to decide when to apply metadata because of how completely arbitrary and unreasonably difficult our ADT handling is (i.e. trans::adt).
Thankfully, that can now be eroded away, and once we have a bit more uniformity we can start doing things better.

That said, we already keep that information so it could be used... oh, huh, we already apply !nonnull.
This is probably a more complex interaction between variant accesses and optimizations then.

[bluss]

Same for Some(&T), affects a lot of iterator optimizations.

[nikic]

I was wondering why this optimizes for me locally, but not on playground ... turns out it optimizes properly at O2, but not at O3 :(

[nikic]

The cause is argument promotion. A pointer argument is promoted to a scalar, and looses the !nonnull metadata in the process.

[bugadani]

Since rustc 1.45 the following IR is emitted at --opt-level=3:

define zeroext i1 @_ZN7example7foo_box17hd63365026be7dd71E(i32** nocapture align 8 dereferenceable(8) %a, i32* noalias nonnull align 4 %b) unnamed_addr #0 personality i32 (i32, i32, i64, %"unwind::libunwind::_Unwind_Exception"*, %"unwind::libunwind::_Unwind_Context"*)* @rust_eh_personality !dbg !29 {
  %0 = ptrtoint i32* %b to i64
  %1 = bitcast i32** %a to i64*, !dbg !30
  store i64 %0, i64* %1, align 8, !dbg !43, !noalias !45
  ret i1 true, !dbg !50
}

[dianqk]

rustc 1.65 has already fixed this issue: https://rust.godbolt.org/z/8cz6rn3v1.

@rustbot label E-needs-test -llvm-fixed-upstream (I used the wrong label.)

[rustbot]

Error: Parsing relabel command in comment failed: ...'d-upstream' | error: a label delta at >| ' (I used t'...

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

[dianqk]

@rustbot label -llvm-fixed-upstream