https://gcc.gnu.org/g:9496224132aa7179219195f218e3a4c7eb13b9ce
commit r17-1854-g9496224132aa7179219195f218e3a4c7eb13b9ce Author: Jeff Law <[email protected]> Date: Thu Jun 25 07:24:49 2026 -0600 [RISC-V][PR target/124019] Add pattern to improve QI comparisons on RISC-V So after much head-banging from Daniel and myself I think it's time to acknowledge this issue isn't great for solving in match.pd. Depending on the target's properties it may be advantageous to handle generating code for this kind of idiom in different ways: It may be better to mask off the irrelevant bits first, then compare to an adjusted constant. It may also be better to do a simpler direct approach of shifting, the comparing to the constant. Worse yet, the desired approach may vary for a given architecture depending on the type of the object (it's related to what bits need to be masked off and the constant we have to generate). While I was able to convince myself we could canonicalize in match.pd, then recover the regressed cases with target work, it'd be a lot of target testing & hacking. The effort is likely equivalent to leaving match.pd alone and adjusting the target patterns to generate good code for the poorly handled cases. ie, in both scenarios we're likely doing significant target work. Some quick evaluation against 502.gcc showed this never shows up in that benchmark. However, it does show up in a trunk bootstrap on RISC-V (given an earlier version with a code generation bug triggered a bootstrap failure). While it's far from wide coverage, I suspect if we were to do wider testing it rarely hits. So I'm taking the conservative approach here. Leave match.pd alone, add a pattern to the RISC-V port to capture this oddball case, then move on. Regression tested on riscv32-elf and riscv64-elf and bootstrap and regression tested on the c920 and K3 designs. Waiting on pre-commit CI before moving forward. PR target/124019 gcc/ * config/riscv/riscv.md (seq_sne_qi): New define_insn_and_split. gcc/testsuite * gcc.target/riscv/pr124019.c: New test. Co-authored-by: Daniel Barboza <[email protected]> Diff: --- gcc/config/riscv/riscv.md | 43 +++++++++++++++++++++++++++++++ gcc/testsuite/gcc.target/riscv/pr124019.c | 37 ++++++++++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md index b90bfe0745a9..d0cfa2c81edf 100644 --- a/gcc/config/riscv/riscv.md +++ b/gcc/config/riscv/riscv.md @@ -5356,6 +5356,49 @@ (set (match_dup 0) (any_eq:X (match_dup 0) (const_int 0)))] { operands[2] = GEN_INT (-UINTVAL (operands[2])); }) +;; So the idea here is to realize that with a single insn we +;; can mask off all the relevant bits in the source operand. +;; A second insn generates zero/non-zero +;; The third and final insn canonicalizes that result to 0/1. +;; +;; There's probably some HImode cases we could handle too. I haven't +;; thought hard about them. +(define_insn_and_split "seq_sne_qi" + [(set (match_operand:X 0 "register_operand" "=r") + (any_eq:X (subreg:QI + (ashift:X (match_operand:X 1 "register_operand" "r") + (match_operand 2 "const_int_operand")) 0) + (match_operand 3 "const_int_operand")))] + "(INTVAL (operands[2]) > 0 && INTVAL (operands[2]) < 8 + && INTVAL (operands[3]) >= -128 && INTVAL (operands[3]) <= 127 + && (INTVAL (operands[3]) + & ((HOST_WIDE_INT_1U << INTVAL (operands[2])) - 1)) == 0)" + "#" + "&& 1" + [(const_int 0)] +{ + operands[3] = GEN_INT (-((INTVAL (operands[3]) & 0xff) + >> INTVAL (operands[2]))); + operands[2] = GEN_INT (0xff >> INTVAL (operands[2])); + + /* We generate code here rather than in the split RTL template so that we + can elide the PLUS if it is not needed. */ + rtx x = gen_rtx_AND (word_mode, operands[1], operands[2]); + emit_insn (gen_rtx_SET (operands[0], x)); + + if (operands[3] != CONST0_RTX (word_mode)) + { + x = gen_rtx_PLUS (word_mode, operands[0], operands[3]); + emit_insn (gen_rtx_SET (operands[0], x)); + } + + x = gen_rtx_fmt_ee (<CODE>, word_mode, operands[0], CONST0_RTX (word_mode)); + emit_insn (gen_rtx_SET (operands[0], x)); + DONE; +} + [(set_attr "type" "slt") + (set_attr "mode" "<X:MODE>")]) + (include "bitmanip.md") (include "crypto.md") (include "sync.md") diff --git a/gcc/testsuite/gcc.target/riscv/pr124019.c b/gcc/testsuite/gcc.target/riscv/pr124019.c new file mode 100644 index 000000000000..4ee56a5bb07c --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/pr124019.c @@ -0,0 +1,37 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -march=rv64gcbv_zicond -mabi=lp64d" { target { rv64 } } } */ +/* { dg-options "-O2 -march=rv32gcbv_zicond -mabi=ilp32" { target { rv32 } } } */ + +#define TEST(TYPE,NAME) _Bool f1_##NAME(TYPE a) { TYPE b = a << 4; return b == -128; } + +TEST (unsigned char, uc); +TEST (signed char, sc); + +TEST (unsigned short, us); +TEST (signed short, ss); + +TEST (unsigned int, ui); +TEST (signed int, si); + +TEST (unsigned long, ul); +TEST (signed long, sl); + +/* The unsigned char and unsigned short cases collapse to a trivial li. */ +/* { dg-final { scan-assembler-times {\tli} 2 } } */ + +/* The signed char case will have an andi. It's the only one. */ +/* { dg-final { scan-assembler-times {\tandi} 1 } } */ +/* { dg-final { scan-assembler-not {\tsext.b} } } */ + +/* Other than the degenerate unsigned char/short cases, every test has + an sll, addi and seqz. */ +/* { dg-final { scan-assembler-times {\taddi} 6 } } */ +/* { dg-final { scan-assembler-times {\tseqz} 6 } } */ + +/* The signed char, and degenerate unsigned tests have slli[w]. */ +/* { dg-final { scan-assembler-times {\tslli} 5 } } */ + +/* The signed short will need an sext. */ +/* { dg-final { scan-assembler-times {\tsext.h} 1 } } */ + +
