Fast isel dislikes switch, which pattern matching can generate

[pcwalton]

We should use the compare mode otherwise.

[huonw]

Triage. As demonstrated below, we still generate switchs, but I don't know how to check for Fast ISel vs not, so I can't verify if they still upset it after 9 months of upgrades.

fn main() {
    match 1 {
        0 => {}
        _ => {}
    };
}

define void @_ZN4main18h7c52aaf9b95dc92af4v0.0E({ i64, %tydesc*, i8*, i8*, i8 }*) #4 {
"function top level":
  %1 = alloca i64
  store i64 1, i64* %1
  %2 = load i64* %1
  switch i64 %2, label %match_else [
    i64 0, label %match_case
  ]

case_body:                                        ; preds = %match_case
  br label %join

case_body1:                                       ; preds = %match_else
  br label %join

match_else:                                       ; preds = %"function top level"
  br label %case_body1

match_case:                                       ; preds = %"function top level"
  br label %case_body

join:                                             ; preds = %case_body1, %case_body
  ret void
}

[pnkfelix]

P-low, not 1.0 blocker.

[steveklabnik]

Today, compiling with rustc -O --emit=llvm-ir hello.rs, I get

define i64 @main(i64, i8**) unnamed_addr #1 {
top:
  %2 = tail call i64 @_ZN2rt10lang_start20he050f8de3bcc02b7VRIE(i8* bitcast (void ()* @_ZN4main20h04abfe0370f26946eaaE to i8*), i64 %0, i8** %1)
  ret i64 %2
}

No more case. Is this resolved, or does some other code still generate poorly?

[huonw]

I think fast isel is only ever attempted to be used when optimisations are off. Using fast isel makes compilation faster and so we want to ensure we're using with opt-level=0 as much as possible. I don't how one can detect if fast isel is being used by LLVM.

[steveklabnik]

Ah, gotcha, that makes sense. Given that it's an optimization, I assumed that it would be with optimizations on. :)

[jonas-schievink]

FastISel still dislikes switch, and the MIR translator generates it as well. Fixing this with MIR is much easier, we could replace unneeded SwitchInts in the SimplifyCfg pass or write another pass for this.

(you can check for FastISel by running the LLVM IR through llc -O0 -fast-isel -fast-isel-abort 1 test.ll - preferrably using the llc compiled from Rust's LLVM submodule, which is located in ./x86_64-unknown-linux-gnu/llvm/Release/bin/llc)

[arielb1]

@jonas-schievink

We may want to do this in the lowering for SwitchInt.

[steveklabnik]

Triage: I'm not aware of any changes, nor anybody working on something like this.

@rust-lang/compiler do you think this ticket is useful?

[nagisa]

This ticket is still useful. That being said, benefits of fixing this are comparatively minor and there are bigger fish to catch.

[scottmcm]

I think this needs a new repro, as the one above appears to have been fixed by #51323: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=5a29683164b1399f1a2418dd9f6718ee

As an attempt, the following does generate a switch:

pub fn foo(c: i32) -> i32 {
    match c {
        0 => 1,
        1 => 7,
        _ => 2,
    }
}

How were you thinking of fixing this? Should there be a opt-level-zero-only MIR optimiztransformation to replace multi-way switchInts with two-way ones and extra blocks? Or did you want it to happen in codegen?

[nikic]

Unfortunately fastisel doesn't support quite a few things that are common in LLVM IR generated by Rust. It's probably not worthwhile to change the codegen for switch if we're quite likely to fail on an invoke or some scalar pair operation anyway.