Unwrapping a Result<u32,!> produces non-trivial code

[scottmcm]

fn foo(x: Result<u32,!>) -> u32 { x.unwrap() }

compiles in release to

define i32 @foo(i64) unnamed_addr #1 personality i32 (i32, i32, i64, %"unwind::libunwind::_Unwind_Exception"*, %"unwind::libunwind::_Unwind_Context"*)* @rust_eh_personality {
start:
  %1 = and i64 %0, 4294967295
  %cond.i = icmp eq i64 %1, 0
  br i1 %cond.i, label %"_ZN47_$LT$core..result..Result$LT$T$C$$u20$E$GT$$GT$6unwrap17h6dfb0b0940cfa89bE.exit", label %bb3.i

bb3.i:                                            ; preds = %start
; call core::result::unwrap_failed
  tail call fastcc void @_ZN4core6result13unwrap_failed17h70f49da6467af679E()
  unreachable

"_ZN47_$LT$core..result..Result$LT$T$C$$u20$E$GT$$GT$6unwrap17h6dfb0b0940cfa89bE.exit": ; preds = %start
  %self.sroa.4.0.extract.shift.i = lshr i64 %0, 32
  %self.sroa.4.0.extract.trunc.i = trunc i64 %self.sroa.4.0.extract.shift.i to i32
  ret i32 %self.sroa.4.0.extract.trunc.i
}

given that the Err variant is !, the type system information should be sufficient for it to become just

  ret i32 %0

Playground repro: https://play.rust-lang.org/?gist=af0170fb612114692ff35dab58c81615&version=nightly&mode=release

[nagisa]

This is generally applicable to all enums, structs etc with uninhabitable fields.

[scottmcm]

Looks like there are two distinct improvement opportunities here:

• enum layout is storing a discriminant even when none is needed. (One- and zero-variant enums correctly don't store a discriminant, but both Result<T, !> and Result<!, !> do.)
• the monomorphized matching in unwrap is unaware that the uninhabited variant is unreachable, and thus is unable to remove the dead code in the arm.

Should I split the issue?

[SimonSapin]

Doesn’t fixing the former require fixing the latter? While the end result is two different kinds of improvement, they’re very related.

[nagisa]

Neither codegen nor layout are really concerned with this issue. Match MIR generation should simply be made aware of the impossibility of the variants. That being said, there’s a caveat that involves unsafe code guidelines. i.e. I’m not aware of any decision to truly consider variants with never type in them as “unused discriminants”.

[scottmcm]

Well, both of these generate the expected branchless code:

fn foo2(x: Result<u32,!>) -> u32 { 
    match x {
        Ok(a) => a,
    }
}

fn foo3(x: Result<u32,!>) -> u32 { 
    match x {
        Ok(a) => a,
        Err(_) => panic!(),
    }
}

Which implies to me that the never type variant is already an unused discriminant. (I updated my previous post to note that it's the monomorphized unwrap that's the problem, not match codegen in general.)

@SimonSapin I figured it'd be possible to update the MIR discriminant and as operations to account for different layout without actually telling the optimizer enough to remove the dead branch. I could easily be wrong, though.

[dhardy]

Another workaround (not mine):

pub fn foo(x: Result<u32,!>) -> u32 { x.unwrap_or_else(|e| e) }

There's still some shift and truncate instructions (Result<u32, !> is 64-bits?), but no branching.

I did some benchmarking using this and found no overhead using Result<T, !> compared to T in this case.

[scottmcm]

Looks like this PR will fix both parts: #45225 (comment)