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.
Jeff
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 @@ (define_insn_and_split ""
&& 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 124ae2c073a..6075b824f47 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 a1b09e865ea..071d7880aeb 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 5e3ef789e42..b3bbee0a761 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -4344,10 +4344,10 @@ (define_insn_and_split ""
(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)))
@@ -4364,10 +4364,10 @@ (define_insn_and_split ""
(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;)
+ ;
+ }
+}
+
