Unnecessary sext when optimizing a switch on riscv

[WaffleLapkin]

Given this llvm-ir (output of rustc, see details in rust-lang/rust#136216):

define dso_local noundef range(i8 1, 9) i8 @opposite_match(i8 noundef zeroext range(i8 1, 9) %0) unnamed_addr {
start:
  %_0 = alloca [1 x i8], align 1
  %x = alloca [1 x i8], align 1
  store i8 %0, ptr %x, align 1
  %1 = load i8, ptr %x, align 1
  %_2 = zext i8 %1 to i32
  switch i32 %_2, label %bb1 [
    i32 1, label %bb5
    i32 2, label %bb4
    i32 4, label %bb3
    i32 8, label %bb2
  ]

bb1:                                              ; preds = %start
  unreachable

bb5:                                              ; preds = %start
  store i8 4, ptr %_0, align 1
  br label %bb6

bb4:                                              ; preds = %start
  store i8 8, ptr %_0, align 1
  br label %bb6

bb3:                                              ; preds = %start
  store i8 1, ptr %_0, align 1
  br label %bb6

bb2:                                              ; preds = %start
  store i8 2, ptr %_0, align 1
  br label %bb6

bb6:                                              ; preds = %bb2, %bb3, %bb4, %bb5
  %2 = load i8, ptr %_0, align 1
  ret i8 %2
}

llvm currently produces this:

opposite_match:
        addi    a0, a0, -1
        slli    a0, a0, 24
        srai    a0, a0, 24
        lui     a1, %hi(.Lswitch.table.opposite_match)
        addi    a1, a1, %lo(.Lswitch.table.opposite_match)
        add     a0, a1, a0
        lbu     a0, 0(a0)
        ret

.Lswitch.table.opposite_match:
        .ascii  "\004\b\004\001\004\004\004\002"

slli + srai is a i8->i32 sign extension, but it's not needed here -- 1 <= a0 <= 8 originally (from the range attribute) and after subtracting 1 it's obviously 0 <= a0 <= 7, so sign extension is the same as zero extension (and thus could be a noop in this case).

sext is added in an InstCombinePass:

 define dso_local noundef range(i8 1, 9) i8 @opposite_match(i8 noundef zeroext range(i8 1, 9) %0) unnamed_addr {
 start:
-  %switch.tableidx = sub nsw i8 %0, 1
-  %switch.gep = getelementptr inbounds [8 x i8], ptr @switch.table.opposite_match, i32 0, i8 %switch.tableidx
+  %switch.tableidx = add nsw i8 %0, -1
+  %1 = sext i8 %switch.tableidx to i32
+  %switch.gep = getelementptr inbounds [8 x i8], ptr @switch.table.opposite_match, i32 0, i32 %1
   %switch.load = load i8, ptr %switch.gep, align 1
   ret i8 %switch.load
 }

Godbolt link (and godbolt link with more versions of the function, which I was experimenting with).

[nikic]

We should fix SimplifyCFG to generate the GEP with the correct index width.

[dtcxzyw]

Reproducer: https://godbolt.org/z/oWf3b9jcr
It is a little weird that i64 is marked as illegal type here...

[nikic]

It is a little weird that i64 is marked as illegal type here...

This report is for the riscv32i-unknown-none-elf target, so i64 not being marked legal is expected.

[dtcxzyw]

It is a little weird that i64 is marked as illegal type here...

This report is for the riscv32i-unknown-none-elf target, so i64 not being marked legal is expected.

Oh, I saw that in the compiler overrides.

[andjo403]

with #146207 the slli + srai is now only a zext.b instruction, think that to remove that instruction the backend needs to know about the range attribute on the function argument but I do not think that information is lowered to the backend and do not know how or if possible to do that,

[dtcxzyw]

with #146207 the slli + srai is now only a zext.b instruction, think that to remove that instruction the backend needs to know about the range attribute on the function argument but I do not think that information is lowered to the backend and do not know how or if possible to do that,

We can eliminate the zext.b by adding nneg to the zext. However, SCCP/CVP runs before the late SimplifyCFG pass, so we don't have the opportunity to add nneg with range info (InstCombine uses KnownBits to infer nneg). Can we directly add nneg in SimplifyCFG? I think it is safe as the default target bb is unreachable.

[nikic]

with #146207 the slli + srai is now only a zext.b instruction, think that to remove that instruction the backend needs to know about the range attribute on the function argument but I do not think that information is lowered to the backend and do not know how or if possible to do that,

We can eliminate the zext.b by adding nneg to the zext. However, SCCP/CVP runs before the late SimplifyCFG pass, so we don't have the opportunity to add nneg with range info (InstCombine uses KnownBits to infer nneg). Can we directly add nneg in SimplifyCFG? I think it is safe as the default target bb is unreachable.

Sounds fine to me. (For the cases where we know it's non-negative, it's not always the case.)