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:
#define TEST(TYPE,NAME) _Bool f1_##NAME(TYPE a) { TYPE b = a << 4;
return b == -128; }
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.
Jeff
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.
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index b90bfe0745a9..16b4ea51f72e 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -5356,6 +5356,50 @@ (define_split
(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 } } */
+
+
