[RISCV] Add isel-patterns to optimize (a < 1) into blez (a <= 0)

[philipp.tomsich]

Summary

The following code-sequence showed up in a testcase (isolated from
SPEC2017) for if-conversion and vectorization when searching for the
maximum in an array:
        addi    a2, zero, 1
        blt     a1, a2, .LBB0_5
which can be expressed as `bge zero,a1,.LBB0_5`/`blez a1,/LBB0_5`.

More generally, we want to express (a < 1) as (a <= 0).

This adds the required isel-pattern and updates the testcases.

[craig.topper]

llvm/lib/Target/RISCV/RISCVInstrInfo.td

996

We already have this in this exact spot in the file. Is your repo out of date?

// Match X > -1, the canonical form of X >= 0, to the bgez pattern.                                                                                                                                                                                                                                                                                                                                                                                                      
def : Pat<(brcond (XLenVT (setgt GPR:$rs1, -1)), bb:$imm12),                                                                                                                                                                                                                                                                                                                                                                                                             
          (BGE GPR:$rs1, X0, bb:$imm12)>;

[philipp.tomsich]

Added a testcase and rebased onto

commit 49942c6d4a0aee740381f754a6a0e6f7e1bfed43
Author: Quentin Colombet <[e-mail]>
Date:   Wed Mar 10 13:28:53 2021 -0800

[jrtc27]

This feels like it should be a target-independent DAGCombine?

[craig.topper]

In D98449#2621957, @jrtc27 wrote:

This feels like it should be a target-independent DAGCombine?

DAG combine and instcombine canonicalize to constants on the RHS and SETGT, SETLT, SETUGT, and SETULT. So X <= 0 will always be rewritten to X< 1.

[jrtc27]

In D98449#2622341, @craig.topper wrote:

In D98449#2621957, @jrtc27 wrote:

This feels like it should be a target-independent DAGCombine?

DAG combine and instcombine canonicalize to constants on the RHS and SETGT, SETLT, SETUGT, and SETULT. So X <= 0 will always be rewritten to X< 1.

Ah ok then this looks good to me. Would be good to have the test pre-committed with the worse codegen as usual though.

[frasercrmck]

llvm/lib/Target/RISCV/RISCVInstrInfo.td

1001

Indentation looks a little off here. Everything else seems to have aligned the to and from.

[craig.topper]

llvm/lib/Target/RISCV/RISCVInstrInfo.td

999

Isn't this the blez pattern?

The "into" doesn't read right after the "as"

Maybe
" Lower (a < 1) as (X0 >= a), the blez pattern."

llvm/test/CodeGen/RISCV/branch.ll

128

This code is pretty trivially unreachable. The block above jumps over it. I'm a little surprised CodeGenPrepare or UnreachableBlockElim didn't notice that. I'd prefer we make it reachable so it can't break in the future.

[craig.topper]

FYI, I put up a related patch to fix this same issue for select https://reviews.llvm.org/D98542

[craig.topper]

llvm/test/CodeGen/RISCV/branch.ll

128

I applied this patch and this test case fails because the unreachable block does get deleted.

[craig.topper]

This patch also appears to affect llvm/test/CodeGen/RISCV/hoist-global-addr-base.ll

[philipp.tomsich]

This updates the test cases and I have rerun 'ninja check' to ensure no further test issues.
The commit message has also been updated to reflect that this is a blez-instruction.

[philipp.tomsich]

Update whitespaces to all-spaces for tabs-and-spaces.

[philipp.tomsich]

Update the comment to reflect that this will turn into a blez instruction.

[craig.topper]

LGTM. Do you need me to commit it? If so can you let me know your email and how you want the Author line to appear in the commit log.

[philipp.tomsich]

Please go ahead and commit this for me.

Thanks,
Philipp.

llvm/lib/Target/RISCV/RISCVInstrInfo.td

 // Extra patterns are needed for a brcond without a setcc (i.e. where the
 // condition was calculated elsewhere).
 def : Pat<(brcond GPR:$cond, bb:$imm12), (BNE GPR:$cond, X0, bb:$imm12)>;
 // In this pattern, the `(xor $cond, 1)` functions like (boolean) `not`, as the
 // `brcond` only uses the lowest bit.
 def : Pat<(brcond (XLenVT (xor GPR:$cond, 1)), bb:$imm12),
           (BEQ GPR:$cond, X0, bb:$imm12)>;
 
 // Match X > -1, the canonical form of X >= 0, to the bgez pattern.
 def : Pat<(brcond (XLenVT (setgt GPR:$rs1, -1)), bb:$imm12),
           (BGE GPR:$rs1, X0, bb:$imm12)>;
+// Lower (a < 1) as (0 >= a) into the blez pattern.
+def : Pat<(brcond (XLenVT (setlt GPR:$lhs, 1)), bb:$imm12),
+          (BGE X0, GPR:$lhs, bb:$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 GPR:$rs1, simm12:$imm12), []>,
                   PseudoInstExpansion<(JALR X0, GPR:$rs1, simm12:$imm12)>;

llvm/test/CodeGen/RISCV/branch.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=riscv32 -verify-machineinstrs < %s \
 ; RUN:   | FileCheck -check-prefix=RV32I %s
 
 define void @foo(i32 %a, i32 *%b, i1 %c) nounwind {
 ; RV32I-LABEL: foo:
 ; RV32I:       # %bb.0:
 ; RV32I-NEXT:    lw a3, 0(a1)
-; RV32I-NEXT:    beq a3, a0, .LBB0_13
+; RV32I-NEXT:    beq a3, a0, .LBB0_14
 ; RV32I-NEXT:  # %bb.1: # %test2
 ; RV32I-NEXT:    lw a3, 0(a1)
-; RV32I-NEXT:    bne a3, a0, .LBB0_13
+; RV32I-NEXT:    bne a3, a0, .LBB0_14
 ; RV32I-NEXT:  # %bb.2: # %test3
 ; RV32I-NEXT:    lw a3, 0(a1)
-; RV32I-NEXT:    blt a3, a0, .LBB0_13
+; RV32I-NEXT:    blt a3, a0, .LBB0_14
 ; RV32I-NEXT:  # %bb.3: # %test4
 ; RV32I-NEXT:    lw a3, 0(a1)
-; RV32I-NEXT:    bge a3, a0, .LBB0_13
+; RV32I-NEXT:    bge a3, a0, .LBB0_14
 ; RV32I-NEXT:  # %bb.4: # %test5
 ; RV32I-NEXT:    lw a3, 0(a1)
-; RV32I-NEXT:    bltu a3, a0, .LBB0_13
+; RV32I-NEXT:    bltu a3, a0, .LBB0_14
 ; RV32I-NEXT:  # %bb.5: # %test6
 ; RV32I-NEXT:    lw a3, 0(a1)
-; RV32I-NEXT:    bgeu a3, a0, .LBB0_13
+; RV32I-NEXT:    bgeu a3, a0, .LBB0_14
 ; RV32I-NEXT:  # %bb.6: # %test7
 ; RV32I-NEXT:    lw a3, 0(a1)
-; RV32I-NEXT:    blt a0, a3, .LBB0_13
+; RV32I-NEXT:    blt a0, a3, .LBB0_14
 ; RV32I-NEXT:  # %bb.7: # %test8
 ; RV32I-NEXT:    lw a3, 0(a1)
-; RV32I-NEXT:    bge a0, a3, .LBB0_13
+; RV32I-NEXT:    bge a0, a3, .LBB0_14
 ; RV32I-NEXT:  # %bb.8: # %test9
 ; RV32I-NEXT:    lw a3, 0(a1)
-; RV32I-NEXT:    bltu a0, a3, .LBB0_13
+; RV32I-NEXT:    bltu a0, a3, .LBB0_14
 ; RV32I-NEXT:  # %bb.9: # %test10
 ; RV32I-NEXT:    lw a3, 0(a1)
-; RV32I-NEXT:    bgeu a0, a3, .LBB0_13
+; RV32I-NEXT:    bgeu a0, a3, .LBB0_14
 ; RV32I-NEXT:  # %bb.10: # %test11
 ; RV32I-NEXT:    lw a0, 0(a1)
 ; RV32I-NEXT:    andi a0, a2, 1
-; RV32I-NEXT:    bnez a0, .LBB0_13
+; RV32I-NEXT:    bnez a0, .LBB0_14
 ; RV32I-NEXT:  # %bb.11: # %test12
 ; RV32I-NEXT:    lw a0, 0(a1)
-; RV32I-NEXT:    bgez a0, .LBB0_13
+; RV32I-NEXT:    bgez a0, .LBB0_14
 ; RV32I-NEXT:  # %bb.12: # %test13
 ; RV32I-NEXT:    lw a0, 0(a1)
-; RV32I-NEXT:  .LBB0_13: # %end
+; RV32I-NEXT:    blez a0, .LBB0_14
+; RV32I-NEXT:  # %bb.13: # %test14
+; RV32I-NEXT:    lw a0, 0(a1)
+; RV32I-NEXT:  .LBB0_14: # %end
 ; RV32I-NEXT:    ret
   %val1 = load volatile i32, i32* %b
   %tst1 = icmp eq i32 %val1, %a
   br i1 %tst1, label %end, label %test2
 
 test2:
   %val2 = load volatile i32, i32* %b
   %tst2 = icmp ne i32 %val2, %a

 
 ; Check that we use bgez for X > -1 which is the canonical form.
 
 test12:
   %val12 = load volatile i32, i32* %b
   %tst12 = icmp sgt i32 %val12, -1
   br i1 %tst12, label %end, label %test13
 
+; Check that we use blez (X <= 0) for (X < 1)
+
 test13:
   %val13 = load volatile i32, i32* %b
+  %tst13 = icmp slt i32 %val13, 1
+  br i1 %tst13, label %end, label %test14
+
+test14:
+  %val14 = load volatile i32, i32* %b
   br label %end
 
 end:
   ret void
 }

llvm/test/CodeGen/RISCV/hoist-global-addr-base.ll

 }
 
 define dso_local void @control_flow_with_mem_access() local_unnamed_addr nounwind {
 ; CHECK-LABEL: control_flow_with_mem_access:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    lui a0, %hi(s)
 ; CHECK-NEXT:    addi a0, a0, %lo(s)
 ; CHECK-NEXT:    lw a1, 164(a0)
-; CHECK-NEXT:    addi a2, zero, 1
-; CHECK-NEXT:    blt a1, a2, .LBB1_2
+; CHECK-NEXT:    blez a1, .LBB1_2
 ; CHECK-NEXT:  # %bb.1: # %if.then
 ; CHECK-NEXT:    addi a1, zero, 10
 ; CHECK-NEXT:    sw a1, 160(a0)
 ; CHECK-NEXT:  .LBB1_2: # %if.end
 ; CHECK-NEXT:    ret
 entry:
   %0 = load i32, i32* getelementptr inbounds (%struct.S, %struct.S* @s, i32 0, i32 2), align 4
   %cmp = icmp sgt i32 %0, 0