|
|
606228 |
commit 6001db79c477b03eacc7e7049560921fb54b7845
|
|
|
606228 |
Author: Richard Sandiford <richard.sandiford@arm.com>
|
|
|
606228 |
Date: Mon Sep 7 20:15:36 2020 +0100
|
|
|
606228 |
|
|
|
606228 |
lra: Avoid cycling on certain subreg reloads [PR96796]
|
|
|
606228 |
|
|
|
606228 |
This PR is about LRA cycling for a reload of the form:
|
|
|
606228 |
|
|
|
606228 |
----------------------------------------------------------------------------
|
|
|
606228 |
Changing pseudo 196 in operand 1 of insn 103 on equiv [r105:DI*0x8+r140:DI]
|
|
|
606228 |
Creating newreg=287, assigning class ALL_REGS to slow/invalid mem r287
|
|
|
606228 |
Creating newreg=288, assigning class ALL_REGS to slow/invalid mem r288
|
|
|
606228 |
103: r203:SI=r288:SI<<0x1+r196:DI#0
|
|
|
606228 |
REG_DEAD r196:DI
|
|
|
606228 |
Inserting slow/invalid mem reload before:
|
|
|
606228 |
316: r287:DI=[r105:DI*0x8+r140:DI]
|
|
|
606228 |
317: r288:SI=r287:DI#0
|
|
|
606228 |
----------------------------------------------------------------------------
|
|
|
606228 |
|
|
|
606228 |
The problem is with r287. We rightly give it a broad starting class of
|
|
|
606228 |
POINTER_AND_FP_REGS (reduced from ALL_REGS by preferred_reload_class).
|
|
|
606228 |
However, we never make forward progress towards narrowing it down to
|
|
|
606228 |
a specific choice of class (POINTER_REGS or FP_REGS).
|
|
|
606228 |
|
|
|
606228 |
I think in practice we rely on two things to narrow a reload pseudo's
|
|
|
606228 |
class down to a specific choice:
|
|
|
606228 |
|
|
|
606228 |
(1) a restricted class is specified when the pseudo is created
|
|
|
606228 |
|
|
|
606228 |
This happens for input address reloads, where the class is taken
|
|
|
606228 |
from the target's chosen base register class. It also happens
|
|
|
606228 |
for simple REG reloads, where the class is taken from the chosen
|
|
|
606228 |
alternative's constraints.
|
|
|
606228 |
|
|
|
606228 |
(2) uses of the reload pseudo as a direct input operand
|
|
|
606228 |
|
|
|
606228 |
In this case get_reload_reg tries to reuse the existing register
|
|
|
606228 |
and narrow its class, instead of creating a new reload pseudo.
|
|
|
606228 |
|
|
|
606228 |
However, neither occurs here. As described above, r287 rightly
|
|
|
606228 |
starts out with a wide choice of class, ultimately derived from
|
|
|
606228 |
ALL_REGS, so we don't get (1). And as the comments in the PR
|
|
|
606228 |
explain, r287 is never used as an input reload, only the subreg is,
|
|
|
606228 |
so we don't get (2):
|
|
|
606228 |
|
|
|
606228 |
----------------------------------------------------------------------------
|
|
|
606228 |
Choosing alt 13 in insn 317: (0) r (1) w {*movsi_aarch64}
|
|
|
606228 |
Creating newreg=291, assigning class FP_REGS to r291
|
|
|
606228 |
317: r288:SI=r291:SI
|
|
|
606228 |
Inserting insn reload before:
|
|
|
606228 |
320: r291:SI=r287:DI#0
|
|
|
606228 |
----------------------------------------------------------------------------
|
|
|
606228 |
|
|
|
606228 |
IMO, in this case we should rely on the reload of r316 to narrow
|
|
|
606228 |
down the class of r278. Currently we do:
|
|
|
606228 |
|
|
|
606228 |
----------------------------------------------------------------------------
|
|
|
606228 |
Choosing alt 7 in insn 316: (0) r (1) m {*movdi_aarch64}
|
|
|
606228 |
Creating newreg=289 from oldreg=287, assigning class GENERAL_REGS to r289
|
|
|
606228 |
316: r289:DI=[r105:DI*0x8+r140:DI]
|
|
|
606228 |
Inserting insn reload after:
|
|
|
606228 |
318: r287:DI=r289:DI
|
|
|
606228 |
---------------------------------------------------
|
|
|
606228 |
|
|
|
606228 |
i.e. we create a new pseudo register r289 and give *that* pseudo
|
|
|
606228 |
GENERAL_REGS instead. This is because get_reload_reg only narrows
|
|
|
606228 |
down the existing class for OP_IN and OP_INOUT, not OP_OUT.
|
|
|
606228 |
|
|
|
606228 |
But if we have a reload pseudo in a reload instruction and have chosen
|
|
|
606228 |
a specific class for the reload pseudo, I think we should simply install
|
|
|
606228 |
it for OP_OUT reloads too, if the class is a subset of the existing class.
|
|
|
606228 |
We will need to pick such a register whatever happens (for r289 in the
|
|
|
606228 |
example above). And as explained in the PR, doing this actually avoids
|
|
|
606228 |
an unnecessary move via the FP registers too.
|
|
|
606228 |
|
|
|
606228 |
The patch is quite aggressive in that it does this for all reload
|
|
|
606228 |
pseudos in all reload instructions. I wondered about reusing the
|
|
|
606228 |
condition for a reload move in in_class_p:
|
|
|
606228 |
|
|
|
606228 |
INSN_UID (curr_insn) >= new_insn_uid_start
|
|
|
606228 |
&& curr_insn_set != NULL
|
|
|
606228 |
&& ((OBJECT_P (SET_SRC (curr_insn_set))
|
|
|
606228 |
&& ! CONSTANT_P (SET_SRC (curr_insn_set)))
|
|
|
606228 |
|| (GET_CODE (SET_SRC (curr_insn_set)) == SUBREG
|
|
|
606228 |
&& OBJECT_P (SUBREG_REG (SET_SRC (curr_insn_set)))
|
|
|
606228 |
&& ! CONSTANT_P (SUBREG_REG (SET_SRC (curr_insn_set)))))))
|
|
|
606228 |
|
|
|
606228 |
but I can't really justify that on first principles. I think we
|
|
|
606228 |
should apply the rule consistently until we have a specific reason
|
|
|
606228 |
for doing otherwise.
|
|
|
606228 |
|
|
|
606228 |
gcc/
|
|
|
606228 |
PR rtl-optimization/96796
|
|
|
606228 |
* lra-constraints.c (in_class_p): Add a default-false
|
|
|
606228 |
allow_all_reload_class_changes_p parameter. Do not treat
|
|
|
606228 |
reload moves specially when the parameter is true.
|
|
|
606228 |
(get_reload_reg): Try to narrow the class of an existing OP_OUT
|
|
|
606228 |
reload if we're reloading a reload pseudo in a reload instruction.
|
|
|
606228 |
|
|
|
606228 |
gcc/testsuite/
|
|
|
606228 |
PR rtl-optimization/96796
|
|
|
606228 |
* gcc.c-torture/compile/pr96796.c: New test.
|
|
|
606228 |
|
|
|
606228 |
diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c
|
|
|
606228 |
index 580da9c3ed6..161b721efb1 100644
|
|
|
606228 |
--- a/gcc/lra-constraints.c
|
|
|
606228 |
+++ b/gcc/lra-constraints.c
|
|
|
606228 |
@@ -236,12 +236,17 @@ get_reg_class (int regno)
|
|
|
606228 |
CL. Use elimination first if REG is a hard register. If REG is a
|
|
|
606228 |
reload pseudo created by this constraints pass, assume that it will
|
|
|
606228 |
be allocated a hard register from its allocno class, but allow that
|
|
|
606228 |
- class to be narrowed to CL if it is currently a superset of CL.
|
|
|
606228 |
+ class to be narrowed to CL if it is currently a superset of CL and
|
|
|
606228 |
+ if either:
|
|
|
606228 |
+
|
|
|
606228 |
+ - ALLOW_ALL_RELOAD_CLASS_CHANGES_P is true or
|
|
|
606228 |
+ - the instruction we're processing is not a reload move.
|
|
|
606228 |
|
|
|
606228 |
If NEW_CLASS is nonnull, set *NEW_CLASS to the new allocno class of
|
|
|
606228 |
REGNO (reg), or NO_REGS if no change in its class was needed. */
|
|
|
606228 |
static bool
|
|
|
606228 |
-in_class_p (rtx reg, enum reg_class cl, enum reg_class *new_class)
|
|
|
606228 |
+in_class_p (rtx reg, enum reg_class cl, enum reg_class *new_class,
|
|
|
606228 |
+ bool allow_all_reload_class_changes_p = false)
|
|
|
606228 |
{
|
|
|
606228 |
enum reg_class rclass, common_class;
|
|
|
606228 |
machine_mode reg_mode;
|
|
|
606228 |
@@ -266,7 +271,8 @@ in_class_p (rtx reg, enum reg_class cl, enum reg_class *new_class)
|
|
|
606228 |
typically moves that have many alternatives, and restricting
|
|
|
606228 |
reload pseudos for one alternative may lead to situations
|
|
|
606228 |
where other reload pseudos are no longer allocatable. */
|
|
|
606228 |
- || (INSN_UID (curr_insn) >= new_insn_uid_start
|
|
|
606228 |
+ || (!allow_all_reload_class_changes_p
|
|
|
606228 |
+ && INSN_UID (curr_insn) >= new_insn_uid_start
|
|
|
606228 |
&& curr_insn_set != NULL
|
|
|
606228 |
&& ((OBJECT_P (SET_SRC (curr_insn_set))
|
|
|
606228 |
&& ! CONSTANT_P (SET_SRC (curr_insn_set)))
|
|
|
606228 |
@@ -551,13 +557,12 @@ init_curr_insn_input_reloads (void)
|
|
|
606228 |
curr_insn_input_reloads_num = 0;
|
|
|
606228 |
}
|
|
|
606228 |
|
|
|
606228 |
-/* Create a new pseudo using MODE, RCLASS, ORIGINAL or reuse already
|
|
|
606228 |
- created input reload pseudo (only if TYPE is not OP_OUT). Don't
|
|
|
606228 |
- reuse pseudo if IN_SUBREG_P is true and the reused pseudo should be
|
|
|
606228 |
- wrapped up in SUBREG. The result pseudo is returned through
|
|
|
606228 |
- RESULT_REG. Return TRUE if we created a new pseudo, FALSE if we
|
|
|
606228 |
- reused the already created input reload pseudo. Use TITLE to
|
|
|
606228 |
- describe new registers for debug purposes. */
|
|
|
606228 |
+/* Create a new pseudo using MODE, RCLASS, ORIGINAL or reuse an existing
|
|
|
606228 |
+ reload pseudo. Don't reuse an existing reload pseudo if IN_SUBREG_P
|
|
|
606228 |
+ is true and the reused pseudo should be wrapped up in a SUBREG.
|
|
|
606228 |
+ The result pseudo is returned through RESULT_REG. Return TRUE if we
|
|
|
606228 |
+ created a new pseudo, FALSE if we reused an existing reload pseudo.
|
|
|
606228 |
+ Use TITLE to describe new registers for debug purposes. */
|
|
|
606228 |
static bool
|
|
|
606228 |
get_reload_reg (enum op_type type, machine_mode mode, rtx original,
|
|
|
606228 |
enum reg_class rclass, bool in_subreg_p,
|
|
|
606228 |
@@ -616,6 +621,35 @@ get_reload_reg (enum op_type type, machine_mode mode, rtx original,
|
|
|
606228 |
|
|
|
606228 |
if (type == OP_OUT)
|
|
|
606228 |
{
|
|
|
606228 |
+ /* Output reload registers tend to start out with a conservative
|
|
|
606228 |
+ choice of register class. Usually this is ALL_REGS, although
|
|
|
606228 |
+ a target might narrow it (for performance reasons) through
|
|
|
606228 |
+ targetm.preferred_reload_class. It's therefore quite common
|
|
|
606228 |
+ for a reload instruction to require a more restrictive class
|
|
|
606228 |
+ than the class that was originally assigned to the reload register.
|
|
|
606228 |
+
|
|
|
606228 |
+ In these situations, it's more efficient to refine the choice
|
|
|
606228 |
+ of register class rather than create a second reload register.
|
|
|
606228 |
+ This also helps to avoid cycling for registers that are only
|
|
|
606228 |
+ used by reload instructions. */
|
|
|
606228 |
+ if (REG_P (original)
|
|
|
606228 |
+ && (int) REGNO (original) >= new_regno_start
|
|
|
606228 |
+ && INSN_UID (curr_insn) >= new_insn_uid_start
|
|
|
606228 |
+ && in_class_p (original, rclass, &new_class, true))
|
|
|
606228 |
+ {
|
|
|
606228 |
+ unsigned int regno = REGNO (original);
|
|
|
606228 |
+ if (lra_dump_file != NULL)
|
|
|
606228 |
+ {
|
|
|
606228 |
+ fprintf (lra_dump_file, " Reuse r%d for output ", regno);
|
|
|
606228 |
+ dump_value_slim (lra_dump_file, original, 1);
|
|
|
606228 |
+ }
|
|
|
606228 |
+ if (new_class != lra_get_allocno_class (regno))
|
|
|
606228 |
+ lra_change_class (regno, new_class, ", change to", false);
|
|
|
606228 |
+ if (lra_dump_file != NULL)
|
|
|
606228 |
+ fprintf (lra_dump_file, "\n");
|
|
|
606228 |
+ *result_reg = original;
|
|
|
606228 |
+ return false;
|
|
|
606228 |
+ }
|
|
|
606228 |
*result_reg
|
|
|
606228 |
= lra_create_new_reg_with_unique_value (mode, original, rclass, title);
|
|
|
606228 |
return true;
|
|
|
606228 |
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr96796.c b/gcc/testsuite/gcc.c-torture/compile/pr96796.c
|
|
|
606228 |
new file mode 100644
|
|
|
606228 |
index 00000000000..8808e62fe77
|
|
|
606228 |
--- /dev/null
|
|
|
606228 |
+++ b/gcc/testsuite/gcc.c-torture/compile/pr96796.c
|
|
|
606228 |
@@ -0,0 +1,55 @@
|
|
|
606228 |
+/* { dg-additional-options "-fcommon" } */
|
|
|
606228 |
+
|
|
|
606228 |
+struct S0 {
|
|
|
606228 |
+ signed f0 : 8;
|
|
|
606228 |
+ unsigned f1;
|
|
|
606228 |
+ unsigned f4;
|
|
|
606228 |
+};
|
|
|
606228 |
+struct S1 {
|
|
|
606228 |
+ long f3;
|
|
|
606228 |
+ char f4;
|
|
|
606228 |
+} g_3_4;
|
|
|
606228 |
+
|
|
|
606228 |
+int g_5, func_1_l_32, func_50___trans_tmp_31;
|
|
|
606228 |
+static struct S0 g_144, g_834, g_1255, g_1261;
|
|
|
606228 |
+
|
|
|
606228 |
+int g_273[120] = {};
|
|
|
606228 |
+int *g_555;
|
|
|
606228 |
+char **g_979;
|
|
|
606228 |
+static int g_1092_0;
|
|
|
606228 |
+static int g_1193;
|
|
|
606228 |
+int safe_mul_func_int16_t_s_s(int si1, int si2) { return si1 * si2; }
|
|
|
606228 |
+static struct S0 *func_50();
|
|
|
606228 |
+int func_1() { func_50(g_3_4, g_5, func_1_l_32, 8, 3); }
|
|
|
606228 |
+void safe_div_func_int64_t_s_s(int *);
|
|
|
606228 |
+void safe_mod_func_uint32_t_u_u(struct S0);
|
|
|
606228 |
+struct S0 *func_50(int p_51, struct S0 p_52, struct S1 p_53, int p_54,
|
|
|
606228 |
+ int p_55) {
|
|
|
606228 |
+ int __trans_tmp_30;
|
|
|
606228 |
+ char __trans_tmp_22;
|
|
|
606228 |
+ short __trans_tmp_19;
|
|
|
606228 |
+ long l_985_1;
|
|
|
606228 |
+ long l_1191[8];
|
|
|
606228 |
+ safe_div_func_int64_t_s_s(g_273);
|
|
|
606228 |
+ __builtin_printf((char*)g_1261.f4);
|
|
|
606228 |
+ safe_mod_func_uint32_t_u_u(g_834);
|
|
|
606228 |
+ g_144.f0 += 1;
|
|
|
606228 |
+ for (;;) {
|
|
|
606228 |
+ struct S1 l_1350 = {&l_1350};
|
|
|
606228 |
+ for (; p_53.f3; p_53.f3 -= 1)
|
|
|
606228 |
+ for (; g_1193 <= 2; g_1193 += 1) {
|
|
|
606228 |
+ __trans_tmp_19 = safe_mul_func_int16_t_s_s(l_1191[l_985_1 + p_53.f3],
|
|
|
606228 |
+ p_55 % (**g_979 = 10));
|
|
|
606228 |
+ __trans_tmp_22 = g_1255.f1 * p_53.f4;
|
|
|
606228 |
+ __trans_tmp_30 = __trans_tmp_19 + __trans_tmp_22;
|
|
|
606228 |
+ if (__trans_tmp_30)
|
|
|
606228 |
+ g_1261.f0 = p_51;
|
|
|
606228 |
+ else {
|
|
|
606228 |
+ g_1255.f0 = p_53.f3;
|
|
|
606228 |
+ int *l_1422 = g_834.f0 = g_144.f4 != (*l_1422)++ > 0 < 0 ^ 51;
|
|
|
606228 |
+ g_555 = ~0;
|
|
|
606228 |
+ g_1092_0 |= func_50___trans_tmp_31;
|
|
|
606228 |
+ }
|
|
|
606228 |
+ }
|
|
|
606228 |
+ }
|
|
|
606228 |
+}
|