Stop forcing an alloc just because something looked at the discriminant

[scottmcm]

The rust code https://rust.godbolt.org/z/5Tvx8Pfqj

pub fn demo(x: Result<i32, u32>) -> isize {
    std::intrinsics::discriminant_value(&x)
}

gives MIR

fn demo(_1: Result<i32, u32>) -> isize {
    debug x => _1;
    let mut _0: isize;

    bb0: {
        _0 = discriminant(_1);
        return;
    }
}

for which the LLVM IR we emit look like

define noundef i64 @demo(i32 noundef range(i32 0, 2) %0, i32 noundef %1) unnamed_addr {
start:
  %x = alloca [8 x i8], align 4
  store i32 %0, ptr %x, align 4
  %2 = getelementptr inbounds i8, ptr %x, i64 4
  store i32 %1, ptr %2, align 4
  %3 = load i32, ptr %x, align 4, !range !3, !noundef !4
  %_0 = zext i32 %3 to i64
  ret i64 %_0
}

Now, LLVM's optimizer is certainly capable of cleaning that up, but it'd be nice if it never needed to in the first place -- especially in debug where the optimizer doesn't run. Since the tag is in %0 as an SSA value already, we could save a bunch of emitting work too.

I think this is caused because the Rvalue::Discriminant is a NonMutatingUseContext::Inspect, which ends up here:

rust/compiler/rustc_codegen_ssa/src/mir/analyze.rs

Lines 231 to 239 in f8a913b

	| PlaceContext::NonMutatingUse(
	NonMutatingUseContext::Inspect
	| NonMutatingUseContext::SharedBorrow
	| NonMutatingUseContext::FakeBorrow
	| NonMutatingUseContext::RawBorrow
	| NonMutatingUseContext::Projection,
	) => {
	self.locals[local] = LocalKind::Memory;
	}

And thus it keeps the variable from being SSA even if it otherwise could.

I don't know what the right fix is. Maybe all the Inspects are fine with SSA, maybe discriminant should be a different kind of use, or maybe the analysis should special-case Rvalue::Discriminant.

[scottmcm]

Or for a more realistic, less-minimized example: https://rust.godbolt.org/z/ae1KWMe1q

pub fn demo(x: Option<u32>) -> u32 {
    if let Some(x) = x { x } else { 0 }
}

that should keep the Option<u32> in SSA the whole time, but it doesn't:

define i32 @demo(i32 %0, i32 %1) unnamed_addr {
start:
  %_0 = alloca [4 x i8], align 4
  %x = alloca [8 x i8], align 4
  store i32 %0, ptr %x, align 4
  %2 = getelementptr inbounds i8, ptr %x, i64 4
  store i32 %1, ptr %2, align 4
  %3 = load i32, ptr %x, align 4
  %_2 = zext i32 %3 to i64
  %4 = icmp eq i64 %_2, 1
  br i1 %4, label %bb1, label %bb2

bb1:
  %5 = getelementptr inbounds i8, ptr %x, i64 4
  %x1 = load i32, ptr %5, align 4
  store i32 %x1, ptr %_0, align 4
  br label %bb3

bb2:
  store i32 0, ptr %_0, align 4
  br label %bb3

bb3:
  %6 = load i32, ptr %_0, align 4
  ret i32 %6

bb4:
  unreachable
}

(The return value being non-SSA is expected, but we shouldn't need to write x to an alloca.)

[saethlin]

In the general case this seems really hard to avoid, because the layout we compute is just that the discriminant has some layout at some offset. Perhaps we can special-case for where the value is all discriminant, or where the value is a ScalarPair and one element of the pair all discriminant?

This has goofy codegen too:

#[no_mangle]
pub fn demo(x: Option<()>) -> u32 {
    if let Some(x) = x { 1 } else { 0 }
}

[scottmcm]

@saethlin Yeah, in the general case the place will be LocalKind::Memory anyway, and if that's the case the discriminant won't change anything. This only matters for things that are Immediate or ScalarPair anyway, as you say.