[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

1001

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

1003

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

[craig.topper]

llvm/lib/Target/RISCV/RISCVInstrInfo.td

1001

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

121 ↗

(On Diff #330217)

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

121 ↗

(On Diff #330217)

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)>;
 
+// Lower (a > -1) to (a >= 0) and (a < 1) to (0 >= a).
+def : Pat<(brcond (XLenVT (setlt XLenVT:$lhs, 1)), bb:$dst),
+    (BGE X0, XLenVT:$lhs, bb:$dst)>;
+def : Pat<(brcond (XLenVT (setgt XLenVT:$lhs, -1)), bb:$dst),
+    (BGE XLenVT:$lhs, X0, bb:$dst)>;
+
 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)>;