Optimize if conditions with integer equality to matches on the integer

[oli-obk]

If we generate the MIR for

fn black_box<T>(t: T) -> T { t }

fn main() {
    let x = black_box(42);
    
    let y = if x == 43 {
        "foo"
    } else {
        "bar"
    };
}

we get something like

        _3 = Eq(move _4, const 43i32);
        StorageDead(_4);
        switchInt(_3) -> [false: bb2, otherwise: bb3];

which we could also write as

        switchInt(_4) -> [43i32: bb3, otherwise: bb2];

(plus move the StorageDead to the beginning of bb2 and bb3). The same goes for != (Ne).

cc @rust-lang/wg-mir-opt

[simonvandel]

I would like to work on this
@rustbot claim

[simonvandel]

Hi @oli-obk
While working on this I see that SwitchInt values are of type u128. This would mean that this optimization could only be done if the integer value is unsigned.

Is it an oversight that SwitchInt does not allow to switch on signed values?

[bjorn3]

The u128 is just a zero-extended version of the value to switch on. For example if i == -1i8 would be something like Switch(i) -> [0xff: bb1, otherwise: bb2].

[tmiasko]

FYI: Codegen does the this transformation in other direction:

rust/src/librustc_codegen_ssa/mir/block.rs

Lines 220 to 224 in 30f0a07

	let switch_llty = bx.immediate_backend_type(bx.layout_of(switch_ty));
	let llval = bx.const_uint_big(switch_llty, values[0]);
	let cmp = bx.icmp(IntPredicate::IntEQ, discr.immediate(), llval);
	helper.maybe_sideeffect(self.mir, &mut bx, targets.as_slice());
	bx.cond_br(cmp, lltrue, llfalse);

[oli-obk]

Hmm... that's good to know. Maybe we should additionally have the opposite optimization as a cleanup before we go into codegen, then this codegen arm can just become an assert.

[bjorn3]

For cg_clif Eq + SwitchInt results in suboptimal codegen over SwitchInt. The former has to materialize the flag into a register before comparing it again, while the later can just perform the comparison and directly jump on the flag. This is because bools have to be represented as integers at the abi boundary and when loading and storing memory. Keeping them as bools until these points results in much more complexity than turning them into integers and compare them in the SwitchInt impl like any other integer. Also it is likely quicker for cg_llvm to switch to icmp + br_if during codegen than it is to rewrite the mir with the associated possibility of a reallocation of the statements vec.

[oli-obk]

so... should we remove that transformation from codegen_ssa and let the backend handle it?

[bjorn3]

I think so. It can wait though, as I don't yet use codegen_ssa anyway.

[simonvandel]

So we still want to do the MIR optimization? I understood from @bjorn3 that switching directly on the integer would be beneficial for cranelift but not necessarily for llvm. Did I get that right?

[bjorn3]

I think that is correct.

[oli-obk]

Yes, we still want this optimization. The llvm backend already handles this change correctly, so I think we should just keep the optimization unconditionally since it simplifies the MIR.