[RISCV][ISel] improved compressed instruction use

[dybv-sc]

Summary

Helping to emit compressed BNEZ in comparasion with constant when possible.

Addressing following issue: https://github.com/llvm/llvm-project/issues/56393

[dybv-sc]

Fixed missing commit

[craig.topper]

As noted in the bug, this increases the critical path length in some cases. Have you benchmarked this?

For immediates that fit in c.li the new sequence might be larger. c.li works for all registers. c.bnez only works for x8-x15 and short distances.

[dybv-sc]

Sorry for long silence.
I've benchmarked SPEC with llvm-test-suite on Alibaba THead machine and found out that there is slight performance downgrade with this substitution. I've isolated one case:

lw      s0, 0(a0)
li      a2, 101
ld      a0, 0(a1)
slliw   a1, s0, 1
addw    a1, a1, s0
sw      a1, 0(a0)
blt     s0, a2, .LBB0_2

transforms to:

lw      s0, 0(a0)
ld      a0, 0(a1)
slti    a2, s0, 101
slliw   a1, s0, 1
addw    a1, a1, s0
sw      a1, 0(a0)
bnez    a2, .LBB0_2

Being put in a hot loop the latter one adds 7 cycles more to each iteration. I found out that It does not affect branch predictor or cache, so there must be a pipeline stall happening here. I'll investigate this further.

[dybv-sc]

So, after more running more spec tests in different modes (train and ref) on different RISCV boards (SiFive and THead) I got mixed results on performance. Performance increase on number on tests was insignificant while on other there was a slight decrease. On average performance declined by 0.5%. On the other hand, size reduction can be seen uniformly among all tests. On average it is 20 less bytes or 0.04% of size reduction. I think these amounts can't justify the performance cost.
Considering that some performance reductions are platform specific (like the one I mentioned in previous comment) and rely on internal architecture features, it is not seem possible to come up with general solution here. And more specialized ones will require more time and effort. And possible 0.04% code size reduction just not worth it.
What do you think?

[craig.topper]

In D132358#3902311, @dybv-sc wrote:

So, after more running more spec tests in different modes (train and ref) on different RISCV boards (SiFive and THead) I got mixed results on performance. Performance increase on number on tests was insignificant while on other there was a slight decrease. On average performance declined by 0.5%. On the other hand, size reduction can be seen uniformly among all tests. On average it is 20 less bytes or 0.04% of size reduction. I think these amounts can't justify the performance cost.
Considering that some performance reductions are platform specific (like the one I mentioned in previous comment) and rely on internal architecture features, it is not seem possible to come up with general solution here. And more specialized ones will require more time and effort. And possible 0.04% code size reduction just not worth it.
What do you think?

Thanks for collecting the data. I agree, it sounds like its not worth it.

llvm/lib/Target/RISCV/RISCVInstrInfo.td

 
 defm : BccPat<SETEQ, BEQ>;
 defm : BccPat<SETNE, BNE>;
 defm : BccPat<SETLT, BLT>;
 defm : BccPat<SETGE, BGE>;
 defm : BccPat<SETULT, BLTU>;
 defm : BccPat<SETUGE, BGEU>;
 
+// Try to produce compressed bnez instruction if possible
+def : Pat<(riscv_brcc GPR:$rs1, simm12:$c, SETLT, bb:$imm12),
+            (BNE (SLTI GPR:$rs1, simm12:$c), X0, simm13_lsb0:$imm12)>;
+
 let isBarrier = 1, isBranch = 1, isTerminator = 1 in
 def PseudoBR : Pseudo<(outs), (ins simm21_lsb0_jal:$imm20), [(br bb:$imm20)]>,
                PseudoInstExpansion<(JAL X0, simm21_lsb0_jal:$imm20)>;
 
 let isBarrier = 1, isBranch = 1, isIndirectBranch = 1, isTerminator = 1 in
 def PseudoBRIND : Pseudo<(outs), (ins GPRJALR:$rs1, simm12:$imm12), []>,
                   PseudoInstExpansion<(JALR X0, GPR:$rs1, simm12:$imm12)>;
 

llvm/test/CodeGen/RISCV/codemodel-lowering.ll

 ; Check lowering of blockaddress that forces a displacement to be added
 
 define signext i32 @lower_blockaddress_displ(i32 signext %w) nounwind {
 ; RV32I-SMALL-LABEL: lower_blockaddress_displ:
 ; RV32I-SMALL:       # %bb.0: # %entry
 ; RV32I-SMALL-NEXT:    addi sp, sp, -16
 ; RV32I-SMALL-NEXT:    lui a1, %hi(.Ltmp0)
 ; RV32I-SMALL-NEXT:    addi a1, a1, %lo(.Ltmp0)
-; RV32I-SMALL-NEXT:    li a2, 101
+; RV32I-SMALL-NEXT:    slti a0, a0, 101
 ; RV32I-SMALL-NEXT:    sw a1, 8(sp)
-; RV32I-SMALL-NEXT:    blt a0, a2, .LBB2_3
+; RV32I-SMALL-NEXT:    bnez a0, .LBB2_3
 ; RV32I-SMALL-NEXT:  # %bb.1: # %if.then
 ; RV32I-SMALL-NEXT:    lw a0, 8(sp)
 ; RV32I-SMALL-NEXT:    jr a0
 ; RV32I-SMALL-NEXT:  .Ltmp0: # Block address taken
 ; RV32I-SMALL-NEXT:  .LBB2_2: # %return
 ; RV32I-SMALL-NEXT:    li a0, 4
 ; RV32I-SMALL-NEXT:    addi sp, sp, 16
 ; RV32I-SMALL-NEXT:    ret
 ; RV32I-SMALL-NEXT:  .LBB2_3: # %return.clone
 ; RV32I-SMALL-NEXT:    li a0, 3
 ; RV32I-SMALL-NEXT:    addi sp, sp, 16
 ; RV32I-SMALL-NEXT:    ret
 ;
 ; RV32I-MEDIUM-LABEL: lower_blockaddress_displ:
 ; RV32I-MEDIUM:       # %bb.0: # %entry
 ; RV32I-MEDIUM-NEXT:    addi sp, sp, -16
 ; RV32I-MEDIUM-NEXT:  .Lpcrel_hi2:
 ; RV32I-MEDIUM-NEXT:    auipc a1, %pcrel_hi(.Ltmp0)
 ; RV32I-MEDIUM-NEXT:    addi a1, a1, %pcrel_lo(.Lpcrel_hi2)
-; RV32I-MEDIUM-NEXT:    li a2, 101
+; RV32I-MEDIUM-NEXT:    slti a0, a0, 101
 ; RV32I-MEDIUM-NEXT:    sw a1, 8(sp)
-; RV32I-MEDIUM-NEXT:    blt a0, a2, .LBB2_3
+; RV32I-MEDIUM-NEXT:    bnez a0, .LBB2_3
 ; RV32I-MEDIUM-NEXT:  # %bb.1: # %if.then
 ; RV32I-MEDIUM-NEXT:    lw a0, 8(sp)
 ; RV32I-MEDIUM-NEXT:    jr a0
 ; RV32I-MEDIUM-NEXT:  .Ltmp0: # Block address taken
 ; RV32I-MEDIUM-NEXT:  .LBB2_2: # %return
 ; RV32I-MEDIUM-NEXT:    li a0, 4
 ; RV32I-MEDIUM-NEXT:    addi sp, sp, 16
 ; RV32I-MEDIUM-NEXT:    ret

llvm/test/CodeGen/RISCV/isel-compressed-comp.ll

+; RUN: llc -mtriple=riscv64 -verify-machineinstrs < %s \
+; RUN:   | FileCheck  %s
+
+declare void @foo()
+
+define void @bar(i64 %a) {
+; CHECK: slti
+; CHECK-NEXT: bnez
+  %c = icmp slt i64  255, %a
+  br i1 %c, label %taken, label %return
+taken:
+  call void @foo()
+  br label %return
+return:
+  ret void
+}