https://gcc.gnu.org/g:36ad5c69a273cb36accd27e70423c81cbd7fbcf9
commit 36ad5c69a273cb36accd27e70423c81cbd7fbcf9
Author: Jeff Law <j...@ventanamicro.com>
Date: Sat Aug 17 09:52:55 2024 -0600
[RISC-V][PR target/116282] Stabilize pattern conditions
So as expected the core problem with target/116282 is that the cost of
certain
constant synthesis cases varied depending on whether or not we're allowed to
generate new pseudos or not.
That in turn meant that in obscure cases an insn might change from
recognizable
to unrecognizable and triggers the observed failure.
So we need to keep the cost stable, at least when called from a pattern's
condition. So we pass another boolean down when necessary. I've tried to
keep
API fallout minimized.
Built and tested on rv32 in my tester. Let's see what pre-commit testing
has
to say though 🙂
Note this will also require a minor change to the in-flight constant
synthesis
work.
PR target/116282
gcc/
* config/riscv/riscv-protos.h (riscv_const_insns): Add new argument.
* config/riscv/riscv.cc (riscv_build_integer): Add new argument
ALLOW_NEW_PSEUDOS. Pass it down to recursive calls and check it
before using synthesis which allows new registers to be created.
(riscv_split_integer_cost): Pass new argument to
riscv_build_integer.
(riscv_integer_cost): Add ALLOW_NEW_PSEUDOS argument, pass it down
to
riscv_build_integer.
(riscv_legitimate_constant_p): Pass new argument to
riscv_const_insns.
(riscv_const_insns): New argment ALLOW_NEW_PSEUDOS. Pass it down to
riscv_integer_cost and riscv_const_insns.
(riscv_split_const_insns): Pass new argument to riscv_const_insns.
(riscv_move_integer, riscv_rtx_costs): Similarly.
* config/riscv/riscv.md (shadd with costly constant): Pass new
argument
to riscv_const_insns.
* config/riscv/bitmanip.md (and with costly constant): Pass new
argument
to riscv_const_insns.
gcc/testsuite/
* gcc.target/riscv/pr116282.c: New test.
(cherry picked from commit 7aed8dedeb9613925930447bf2457c3fd9972d91)
Diff:
---
gcc/config/riscv/bitmanip.md | 2 +-
gcc/config/riscv/riscv-protos.h | 2 +-
gcc/config/riscv/riscv.cc | 66 +++++++++++++++++++++----------
gcc/config/riscv/riscv.md | 16 ++++----
gcc/testsuite/gcc.target/riscv/pr116282.c | 16 ++++++++
5 files changed, 71 insertions(+), 31 deletions(-)
diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md
index 6872ee56022..06ff698bfe7 100644
--- a/gcc/config/riscv/bitmanip.md
+++ b/gcc/config/riscv/bitmanip.md
@@ -1078,7 +1078,7 @@
&& TARGET_ZBA
&& !paradoxical_subreg_p (operands[1])
/* Only profitable if synthesis takes more than one insn. */
- && riscv_const_insns (operands[2]) != 1
+ && riscv_const_insns (operands[2], false) != 1
/* We need the upper half to be zero. */
&& (INTVAL (operands[2]) & HOST_WIDE_INT_C (0xffffffff00000000)) == 0
/* And the the adjusted constant must either be something we can
diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
index f8fc2874cbb..926899ccad6 100644
--- a/gcc/config/riscv/riscv-protos.h
+++ b/gcc/config/riscv/riscv-protos.h
@@ -112,7 +112,7 @@ extern bool riscv_valid_base_register_p (rtx, machine_mode,
bool);
extern enum reg_class riscv_index_reg_class ();
extern int riscv_regno_ok_for_index_p (int);
extern int riscv_address_insns (rtx, machine_mode, bool);
-extern int riscv_const_insns (rtx);
+extern int riscv_const_insns (rtx, bool);
extern int riscv_split_const_insns (rtx);
extern int riscv_load_store_insns (rtx, rtx_insn *);
extern rtx riscv_emit_move (rtx, rtx);
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 420037885be..3e2a6fac2aa 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -1074,11 +1074,16 @@ riscv_build_integer_1 (struct riscv_integer_op
codes[RISCV_MAX_INTEGER_OPS],
}
/* Fill CODES with a sequence of rtl operations to load VALUE.
- Return the number of operations needed. */
+ Return the number of operations needed.
+
+ ALLOW_NEW_PSEUDOS indicates if or caller wants to allow new pseudo
+ registers or not. This is needed for cases where the integer synthesis and
+ costing code are used in insn conditions, we can't have costing allow
+ recognition at some points and reject at others. */
static int
riscv_build_integer (struct riscv_integer_op *codes, HOST_WIDE_INT value,
- machine_mode mode)
+ machine_mode mode, bool allow_new_pseudos)
{
int cost = riscv_build_integer_1 (codes, value, mode);
@@ -1129,7 +1134,8 @@ riscv_build_integer (struct riscv_integer_op *codes,
HOST_WIDE_INT value,
int alt_cost;
HOST_WIDE_INT nval = ~value;
- alt_cost = 1 + riscv_build_integer (alt_codes, nval, mode);
+ alt_cost = 1 + riscv_build_integer (alt_codes, nval,
+ mode, allow_new_pseudos);
if (alt_cost < cost)
{
alt_codes[alt_cost - 1].code = XOR;
@@ -1185,7 +1191,7 @@ riscv_build_integer (struct riscv_integer_op *codes,
HOST_WIDE_INT value,
using zbkb. We may do better than that if the upper or lower half
can be synthesized with a single LUI, ADDI or BSET. Regardless the
basic steps are the same. */
- if (cost > 3 && can_create_pseudo_p ())
+ if (cost > 3 && can_create_pseudo_p () && allow_new_pseudos)
{
struct riscv_integer_op hi_codes[RISCV_MAX_INTEGER_OPS];
struct riscv_integer_op lo_codes[RISCV_MAX_INTEGER_OPS];
@@ -1238,20 +1244,28 @@ riscv_split_integer_cost (HOST_WIDE_INT val)
unsigned HOST_WIDE_INT hival = sext_hwi ((val - loval) >> 32, 32);
struct riscv_integer_op codes[RISCV_MAX_INTEGER_OPS];
- cost = 2 + riscv_build_integer (codes, loval, VOIDmode);
+ /* This routine isn't used by pattern conditions, so whether or
+ not to allow new pseudos can be a function of where we are in the
+ RTL pipeline. We shouldn't need scratch pseudos for this case
+ anyway. */
+ bool allow_new_pseudos = can_create_pseudo_p ();
+ cost = 2 + riscv_build_integer (codes, loval, VOIDmode, allow_new_pseudos);
if (loval != hival)
- cost += riscv_build_integer (codes, hival, VOIDmode);
+ cost += riscv_build_integer (codes, hival, VOIDmode, allow_new_pseudos);
return cost;
}
-/* Return the cost of constructing the integer constant VAL. */
+/* Return the cost of constructing the integer constant VAL. ALLOW_NEW_PSEUDOS
+ potentially restricts if riscv_build_integer is allowed to create new
+ pseudo registers. It must be false for calls directly or indirectly from
+ conditions in patterns. */
static int
-riscv_integer_cost (HOST_WIDE_INT val)
+riscv_integer_cost (HOST_WIDE_INT val, bool allow_new_pseudos)
{
struct riscv_integer_op codes[RISCV_MAX_INTEGER_OPS];
- return MIN (riscv_build_integer (codes, val, VOIDmode),
+ return MIN (riscv_build_integer (codes, val, VOIDmode, allow_new_pseudos),
riscv_split_integer_cost (val));
}
@@ -1528,7 +1542,9 @@ riscv_float_const_rtx_index_for_fli (rtx x)
static bool
riscv_legitimate_constant_p (machine_mode mode ATTRIBUTE_UNUSED, rtx x)
{
- return riscv_const_insns (x) > 0;
+ /* With the post-reload usage, it seems best to just pass in FALSE
+ rather than pass ALLOW_NEW_PSEUDOS through the call chain. */
+ return riscv_const_insns (x, false) > 0;
}
/* Implement TARGET_CANNOT_FORCE_CONST_MEM.
@@ -2076,10 +2092,14 @@ riscv_address_insns (rtx x, machine_mode mode, bool
might_split_p)
}
/* Return the number of instructions needed to load constant X.
- Return 0 if X isn't a valid constant. */
+ Return 0 if X isn't a valid constant.
+
+ ALLOW_NEW_PSEUDOS controls whether or not we're going to be allowed
+ to create new pseduos. It must be FALSE for any call directly or
+ indirectly from a pattern's condition. */
int
-riscv_const_insns (rtx x)
+riscv_const_insns (rtx x, bool allow_new_pseudos)
{
enum riscv_symbol_type symbol_type;
rtx offset;
@@ -2096,7 +2116,7 @@ riscv_const_insns (rtx x)
case CONST_INT:
{
- int cost = riscv_integer_cost (INTVAL (x));
+ int cost = riscv_integer_cost (INTVAL (x), allow_new_pseudos);
/* Force complicated constants to memory. */
return cost < 4 ? cost : 0;
}
@@ -2153,7 +2173,7 @@ riscv_const_insns (rtx x)
scalar register. Loading of a floating-point
constant incurs a literal-pool access. Allow this in
order to increase vectorization possibilities. */
- int n = riscv_const_insns (elt);
+ int n = riscv_const_insns (elt, allow_new_pseudos);
if (CONST_DOUBLE_P (elt))
return 1 + 4; /* vfmv.v.f + memory access. */
else
@@ -2181,9 +2201,9 @@ riscv_const_insns (rtx x)
split_const (x, &x, &offset);
if (offset != 0)
{
- int n = riscv_const_insns (x);
+ int n = riscv_const_insns (x, allow_new_pseudos);
if (n != 0)
- return n + riscv_integer_cost (INTVAL (offset));
+ return n + riscv_integer_cost (INTVAL (offset), allow_new_pseudos);
}
return 0;
@@ -2211,8 +2231,12 @@ riscv_split_const_insns (rtx x)
{
unsigned int low, high;
- low = riscv_const_insns (riscv_subword (x, false));
- high = riscv_const_insns (riscv_subword (x, true));
+ /* This is not called from pattern conditions, so we can let
+ our location in the RTL pipeline control whether or not
+ new pseudos are created. */
+ bool allow_new_pseudos = can_create_pseudo_p ();
+ low = riscv_const_insns (riscv_subword (x, false), allow_new_pseudos);
+ high = riscv_const_insns (riscv_subword (x, true), allow_new_pseudos);
gcc_assert (low > 0 && high > 0);
return low + high;
}
@@ -2736,7 +2760,7 @@ riscv_move_integer (rtx temp, rtx dest, HOST_WIDE_INT
value,
mode = GET_MODE (dest);
/* We use the original mode for the riscv_build_integer call, because HImode
values are given special treatment. */
- num_ops = riscv_build_integer (codes, value, orig_mode);
+ num_ops = riscv_build_integer (codes, value, orig_mode, can_create_pseudo_p
());
if (can_create_pseudo_p () && num_ops > 2 /* not a simple constant */
&& num_ops >= riscv_split_integer_cost (value))
@@ -3565,7 +3589,7 @@ riscv_rtx_costs (rtx x, machine_mode mode, int
outer_code, int opno ATTRIBUTE_UN
case CONST:
/* Non trivial CONST_INT Fall through: check if need multiple insns. */
- if ((cost = riscv_const_insns (x)) > 0)
+ if ((cost = riscv_const_insns (x, can_create_pseudo_p ())) > 0)
{
/* 1. Hoist will GCSE constants only if TOTAL returned is non-zero.
2. For constants loaded more than once, the approach so far has
@@ -4693,7 +4717,7 @@ riscv_emit_int_compare (enum rtx_code *code, rtx *op0,
rtx *op1,
new_rhs = rhs + (increment ? 1 : -1);
new_rhs = trunc_int_for_mode (new_rhs, GET_MODE (*op0));
- if (riscv_integer_cost (new_rhs) < riscv_integer_cost (rhs)
+ if (riscv_integer_cost (new_rhs, true) < riscv_integer_cost (rhs,
true)
&& (rhs < 0) == (new_rhs < 0))
{
*op1 = GEN_INT (new_rhs);
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index f8d8162c0f9..55027800534 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -4407,10 +4407,10 @@
(match_operand 3 "const_int_operand" "n")))
(clobber (match_scratch:DI 4 "=&r"))]
"(TARGET_64BIT
- && riscv_const_insns (operands[3])
- && ((riscv_const_insns (operands[3])
- < riscv_const_insns (GEN_INT (INTVAL (operands[3]) >> INTVAL
(operands[2]))))
- || riscv_const_insns (GEN_INT (INTVAL (operands[3]) >> INTVAL
(operands[2]))) == 0))"
+ && riscv_const_insns (operands[3], false)
+ && ((riscv_const_insns (operands[3], false)
+ < riscv_const_insns (GEN_INT (INTVAL (operands[3]) >> INTVAL
(operands[2])), false))
+ || riscv_const_insns (GEN_INT (INTVAL (operands[3]) >> INTVAL
(operands[2])), false) == 0))"
"#"
"&& reload_completed"
[(set (match_dup 0) (ashift:DI (match_dup 1) (match_dup 2)))
@@ -4427,10 +4427,10 @@
(match_operand 3 "const_int_operand" "n"))))
(clobber (match_scratch:DI 4 "=&r"))]
"(TARGET_64BIT
- && riscv_const_insns (operands[3])
- && ((riscv_const_insns (operands[3])
- < riscv_const_insns (GEN_INT (INTVAL (operands[3]) >> INTVAL
(operands[2]))))
- || riscv_const_insns (GEN_INT (INTVAL (operands[3]) >> INTVAL
(operands[2]))) == 0))"
+ && riscv_const_insns (operands[3], false)
+ && ((riscv_const_insns (operands[3], false)
+ < riscv_const_insns (GEN_INT (INTVAL (operands[3]) >> INTVAL
(operands[2])), false))
+ || riscv_const_insns (GEN_INT (INTVAL (operands[3]) >> INTVAL
(operands[2])), false) == 0))"
"#"
"&& reload_completed"
[(set (match_dup 0) (ashift:DI (match_dup 1) (match_dup 2)))
diff --git a/gcc/testsuite/gcc.target/riscv/pr116282.c
b/gcc/testsuite/gcc.target/riscv/pr116282.c
new file mode 100644
index 00000000000..f86adee644d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/pr116282.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=rv64gc_zba_zbkb -mabi=lp64d" } */
+short a;
+long b;
+char c, d;
+void e(int f[][4][24][4], long g[][24][24][24]) {
+ for (unsigned h = 2;; h = 3)
+ for (long i = 0; i < 4; i = 5006368639)
+ for (int j = 0; j < 4; j = 4) {
+ for (long k = -4294967294; k < (c ?: f[0][2][6][j]); k += b)
+ a = g[k][j][0][h];
+ for (; f ? d : 0;)
+ ;
+ }
+}
+
