On Fri, 31 May 2024 08:07:11 PDT (-0700), Robin Dapp wrote:
Hi,
ifcvt likes to emit
(set
(if_then_else)
(ge (reg 1) (reg2))
(reg 1)
(reg 2))
which can be recognized as min/max patterns in the backend.
This patch adds such patterns and the respective iterators as well as a
test.
This depends on the generic ifcvt change.
https://inbox.sourceware.org/gcc-patches/57bb6ce5-79c3-4b08-b524-e807b9ac4...@gmail.com/T/#u
in case anyone's looking for it.
Regtested on rv64gcv_zvfh_zicond_zbb_zvbb.
Regards
Robin
gcc/ChangeLog:
* config/riscv/bitmanip.md (*<bitmanip_minmax_cmp_insn>_cmp_<mode>3):
New min/max ifcvt pattern.
* config/riscv/iterators.md (minu): New iterator.
* config/riscv/riscv.cc (riscv_noce_conversion_profitable_p):
Remove riscv-specific adjustment.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/zbb-min-max-04.c: New test.
---
gcc/config/riscv/bitmanip.md | 13 +++++
gcc/config/riscv/iterators.md | 8 ++++
gcc/config/riscv/riscv.cc | 3 --
.../gcc.target/riscv/zbb-min-max-04.c | 47 +++++++++++++++++++
4 files changed, 68 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c
diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md
index 8769a6b818b..11102985796 100644
--- a/gcc/config/riscv/bitmanip.md
+++ b/gcc/config/riscv/bitmanip.md
@@ -547,6 +547,19 @@ (define_insn "*<bitmanip_optab><mode>3"
"<bitmanip_insn>\t%0,%1,%z2"
[(set_attr "type" "<bitmanip_insn>")])
+;; Provide a minmax pattern for ifcvt to match.
+(define_insn "*<bitmanip_minmax_cmp_insn>_cmp_<mode>3"
+ [(set (match_operand:X 0 "register_operand" "=r")
+ (if_then_else:X
+ (bitmanip_minmax_cmp_op
+ (match_operand:X 1 "register_operand" "r")
+ (match_operand:X 2 "register_operand" "r"))
+ (match_dup 1)
+ (match_dup 2)))]
+ "TARGET_ZBB"
+ "<bitmanip_minmax_cmp_insn>\t%0,%1,%z2"
+ [(set_attr "type" "<bitmanip_minmax_cmp_insn>")])
This is a bit different than how we're handling the other min/max type
attributes
(define_insn "*<bitmanip_optab><mode>3"
[(set (match_operand:X 0 "register_operand" "=r")
(bitmanip_minmax:X (match_operand:X 1 "register_operand" "r")
(match_operand:X 2 "reg_or_0_operand" "rJ")))]
"TARGET_ZBB"
"<bitmanip_insn>\t%0,%1,%z2"
[(set_attr "type" "<bitmanip_insn>")])
but it looks like it ends up with the same types after all the iterators
(there's some "max vs smax" and "smax vs maxs" juggling, but IIUC it
ends up in the same place). I think it'd be clunkier to try and use all
the same iterators, though, so
Reviewed-by: Palmer Dabbelt <pal...@rivosinc.com>
[I was wondering if we need the reversed, Jeff on the call says we
don't. I couldn't figure out how to write a test for it.]
+
;; Optimize the common case of a SImode min/max against a constant
;; that is safe both for sign- and zero-extension.
(define_insn_and_split "*minmax"
diff --git a/gcc/config/riscv/iterators.md b/gcc/config/riscv/iterators.md
index 8a9d1986b4a..2f7be6e83c1 100644
--- a/gcc/config/riscv/iterators.md
+++ b/gcc/config/riscv/iterators.md
@@ -202,6 +202,14 @@ (define_code_iterator bitmanip_bitwise [and ior])
(define_code_iterator bitmanip_minmax [smin umin smax umax])
+(define_code_iterator bitmanip_minmax_cmp_op [lt ltu le leu ge geu gt gtu])
+
+; Map a comparison operator to a min or max.
+(define_code_attr bitmanip_minmax_cmp_insn [(lt "min") (ltu "minu")
+ (le "min") (leu "minu")
+ (ge "max") (geu "maxu")
+ (gt "max") (gtu "maxu")])
+
(define_code_iterator clz_ctz_pcnt [clz ctz popcount])
(define_code_iterator bitmanip_rotate [rotate rotatert])
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 13cd61a4a22..d17c0a260a2 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -4009,9 +4009,6 @@ riscv_noce_conversion_profitable_p (rtx_insn *seq,
{
struct noce_if_info riscv_if_info = *if_info;
- riscv_if_info.original_cost -= COSTS_N_INSNS (2);
- riscv_if_info.original_cost += insn_cost (if_info->jump, if_info->speed_p);
-
/* Hack alert! When `noce_try_store_flag_mask' uses `cstore<mode>4'
to emit a conditional set operation on DImode output it comes up
with a sequence such as:
diff --git a/gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c
b/gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c
new file mode 100644
index 00000000000..ebf1889075d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/zbb-min-max-04.c
@@ -0,0 +1,47 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=rv64gc_zicond_zbb -mabi=lp64d" } */
+/* { dg-skip-if "" { *-*-* } { "-finline-functions" "-funroll-loops"
"-ftracer" } } */
+
+typedef int move_s;
IIUC that typedef isn't used?
+
+int
+remove_one_fast (int *move_ordering, const int num_moves, int mark)
+{
+ int i, best = -1000000;
+ int tmp = 0;
+
+ for (i = mark; i < num_moves; i++)
+ {
+ if (move_ordering[i] > best)
+ {
+ best = move_ordering[i];
+ tmp = i;
+ }
+ }
+
+ return tmp;
+}
+
+/* { dg-final { scan-assembler-times "max\t" 1 } } */
+/* { dg-final { scan-assembler-times "czero.nez" 2 } } */
+/* { dg-final { scan-assembler-times "czero.eqz" 2 } } */
+
+int
+remove_one_fast2 (int *move_ordering, const int num_moves, int mark)
+{
+ int i, best = -1000000;
+ int tmp = 0;
+
+ for (i = mark; i < num_moves; i++)
+ {
+ if (move_ordering[i] < best)
+ {
+ best = move_ordering[i];
+ tmp = i;
+ }
+ }
+
+ return tmp;
+}
+
+/* { dg-final { scan-assembler-times "min\t" 1 } } */
