PR 60604 shows a case where CANNOT_CHANGE_MODE_CLASS is being ignored
for a subreg of a floating-point register, causing it to be replaced
with an invalid (reg ...).
There are various reasons why MIPS floating-point registers can't change
mode, but one important one for big-endian 32-bit targets is that the
registers are always little-endian. This means an SImode subreg of
a DFmode value refers to the opposite half from what GCC expects.
(This could be fixed by renumbering the registers internally, but since
the FPRs can't change mode for other reasons, it doesn't really seem
worth it.)
Before IRA we have:
(set (reg:DF 44 $f12)
(reg/v:DF 205 [ x ]))
(set (subreg:SI (reg:DF 199 [ D.3267 ]) 0)
(and:SI (subreg:SI (reg/v:DF 205 [ x ]) 0)
(const_int 2147483647 [0x7fffffff])))
(set (subreg:SI (reg:DF 199 [ D.3267 ]) 4)
(subreg:SI (reg/v:DF 205 [ x ]) 4))
IRA changes this to:
(set (reg:DF 44 $f12)
(reg/v:DF 205 [ x ]))
(set (subreg:SI (reg:DF 199 [ D.3267 ]) 0)
(and:SI (subreg:SI (reg:DF 44 $f12) 0) <----
(const_int 2147483647 [0x7fffffff])))
(set (subreg:SI (reg:DF 199 [ D.3267 ]) 4)
(subreg:SI (reg:DF 44 $f12) 4)) <----
And reload creates the following reloads:
Reload 0: reload_in (SI) = (reg:SI 44 $f12)
GR_REGS, RELOAD_FOR_INPUT (opnum = 1), can't combine
reload_in_reg: (subreg:SI (reg:DF 44 $f12) 0)
reload_reg_rtx: (reg:SI 2 $2)
...
Reload 0: reload_in (DF) = (reg:DF 44 $f12)
GR_REGS, RELOAD_FOR_INPUT (opnum = 1)
reload_in_reg: (reg:DF 44 $f12)
reload_reg_rtx: (reg:DF 2 $2)
The second entry, for the lowpart (subreg:SI (reg:DF 44 $f12) 4),
correctly reloads the full $f12 value into a GPR. But the first entry,
for the highpart (subreg:SI (reg:DF 44 $f12) 0), gets simplified to
(reg:SI $f12) in spite of CANNOT_CHANGE_MODE_CLASS.
The difference comes from the fact that push_reload handles lowpart
subregs differently from others. The lowpart code is:
if (in != 0 && GET_CODE (in) == SUBREG
&& (subreg_lowpart_p (in) || strict_low)
&& ([...gruesome condition...]
#ifdef CANNOT_CHANGE_MODE_CLASS
|| (REG_P (SUBREG_REG (in))
&& REGNO (SUBREG_REG (in)) < FIRST_PSEUDO_REGISTER
&& REG_CANNOT_CHANGE_MODE_P
(REGNO (SUBREG_REG (in)), GET_MODE (SUBREG_REG (in)), inmode))
#endif
))
{
...
}
with others being handled by:
if (in != 0 && reload_inner_reg_of_subreg (in, inmode, false))
{
...
}
where reload_inner_reg_of_subreg doesn't check CANNOT_CHANGE_MODE_CLASS.
Simply adding:
#ifdef CANNOT_CHANGE_MODE_CLASS
if (REG_CANNOT_CHANGE_MODE_P (REGNO (inner), GET_MODE (inner), mode))
return true;
#endif
to it isn't enough though, since that just forces the inner subreg
to be reloaded into another FPR and then we simplify _that_ FPR to
an SImode and reload it.
While this is arguably a bug in reload, the situation isn't really
supposed to happen in practice, since register_operand specifically
disallows this kind of subreg:
#ifdef CANNOT_CHANGE_MODE_CLASS
if (REG_P (sub)
&& REGNO (sub) < FIRST_PSEUDO_REGISTER
&& REG_CANNOT_CHANGE_MODE_P (REGNO (sub), GET_MODE (sub), mode)
&& GET_MODE_CLASS (GET_MODE (sub)) != MODE_COMPLEX_INT
&& GET_MODE_CLASS (GET_MODE (sub)) != MODE_COMPLEX_FLOAT
/* LRA can generate some invalid SUBREGS just for matched
operand reload presentation. LRA needs to treat them as
valid. */
&& ! LRA_SUBREG_P (op))
return 0;
#endif
The problem is that nonmemory_operand and general_operand both have
slightly different ideas about what a register operand should be and
neither of them has this check. register_operand also has:
/* FLOAT_MODE subregs can't be paradoxical. Combine will occasionally
create such rtl, and we must reject it. */
if (SCALAR_FLOAT_MODE_P (GET_MODE (op))
/* LRA can use subreg to store a floating point value in an
integer mode. Although the floating point and the
integer modes need the same number of hard registers, the
size of floating point mode can be less than the integer
mode. */
&& ! lra_in_progress
&& GET_MODE_SIZE (GET_MODE (op)) > GET_MODE_SIZE (GET_MODE (sub)))
return 0;
which is copied in general_operand but missing from nonmemory_operand.
This second check doesn't apply to the testcase but it seems unlikely
that the difference is intentional.
This patch therefore consolidates all the register checks in general_operand
and makes the others call into it. Tested on x86_64-linux-gnu and
mips64-linux-gnu. OK to install?
The original testcase was from the fortran testsuite, so no new one here.
Thanks,
Richard
gcc/
PR rtl-optimization/60604
* recog.c (general_operand): Incorporate REG_CANNOT_CHANGE_MODE_P
check from register_operand.
(register_operand): Redefine in terms of general_operand.
(nonmemory_operand): Use register_operand for the non-constant cases.
Index: gcc/recog.c
===================================================================
--- gcc/recog.c 2014-03-30 22:18:23.803009353 +0100
+++ gcc/recog.c 2014-03-30 22:37:47.534721724 +0100
@@ -1023,6 +1023,19 @@ general_operand (rtx op, enum machine_mo
&& MEM_P (sub))
return 0;
+#ifdef CANNOT_CHANGE_MODE_CLASS
+ if (REG_P (sub)
+ && REGNO (sub) < FIRST_PSEUDO_REGISTER
+ && REG_CANNOT_CHANGE_MODE_P (REGNO (sub), GET_MODE (sub), mode)
+ && GET_MODE_CLASS (GET_MODE (sub)) != MODE_COMPLEX_INT
+ && GET_MODE_CLASS (GET_MODE (sub)) != MODE_COMPLEX_FLOAT
+ /* LRA can generate some invalid SUBREGS just for matched
+ operand reload presentation. LRA needs to treat them as
+ valid. */
+ && ! LRA_SUBREG_P (op))
+ return 0;
+#endif
+
/* FLOAT_MODE subregs can't be paradoxical. Combine will occasionally
create such rtl, and we must reject it. */
if (SCALAR_FLOAT_MODE_P (GET_MODE (op))
@@ -1083,9 +1096,6 @@ address_operand (rtx op, enum machine_mo
int
register_operand (rtx op, enum machine_mode mode)
{
- if (GET_MODE (op) != mode && mode != VOIDmode)
- return 0;
-
if (GET_CODE (op) == SUBREG)
{
rtx sub = SUBREG_REG (op);
@@ -1096,41 +1106,12 @@ register_operand (rtx op, enum machine_m
(Ideally, (SUBREG (MEM)...) should not exist after reload,
but currently it does result from (SUBREG (REG)...) where the
reg went on the stack.) */
- if (! reload_completed && MEM_P (sub))
- return general_operand (op, mode);
-
-#ifdef CANNOT_CHANGE_MODE_CLASS
- if (REG_P (sub)
- && REGNO (sub) < FIRST_PSEUDO_REGISTER
- && REG_CANNOT_CHANGE_MODE_P (REGNO (sub), GET_MODE (sub), mode)
- && GET_MODE_CLASS (GET_MODE (sub)) != MODE_COMPLEX_INT
- && GET_MODE_CLASS (GET_MODE (sub)) != MODE_COMPLEX_FLOAT
- /* LRA can generate some invalid SUBREGS just for matched
- operand reload presentation. LRA needs to treat them as
- valid. */
- && ! LRA_SUBREG_P (op))
- return 0;
-#endif
-
- /* FLOAT_MODE subregs can't be paradoxical. Combine will occasionally
- create such rtl, and we must reject it. */
- if (SCALAR_FLOAT_MODE_P (GET_MODE (op))
- /* LRA can use subreg to store a floating point value in an
- integer mode. Although the floating point and the
- integer modes need the same number of hard registers, the
- size of floating point mode can be less than the integer
- mode. */
- && ! lra_in_progress
- && GET_MODE_SIZE (GET_MODE (op)) > GET_MODE_SIZE (GET_MODE (sub)))
+ if (!REG_P (sub) && (reload_completed || !MEM_P (sub)))
return 0;
-
- op = sub;
}
-
- return (REG_P (op)
- && (REGNO (op) >= FIRST_PSEUDO_REGISTER
- || in_hard_reg_set_p (operand_reg_set,
- GET_MODE (op), REGNO (op))));
+ else if (!REG_P (op))
+ return 0;
+ return general_operand (op, mode);
}
/* Return 1 for a register in Pmode; ignore the tested mode. */
@@ -1232,27 +1213,7 @@ nonmemory_operand (rtx op, enum machine_
{
if (CONSTANT_P (op))
return immediate_operand (op, mode);
-
- if (GET_MODE (op) != mode && mode != VOIDmode)
- return 0;
-
- if (GET_CODE (op) == SUBREG)
- {
- /* Before reload, we can allow (SUBREG (MEM...)) as a register operand
- because it is guaranteed to be reloaded into one.
- Just make sure the MEM is valid in itself.
- (Ideally, (SUBREG (MEM)...) should not exist after reload,
- but currently it does result from (SUBREG (REG)...) where the
- reg went on the stack.) */
- if (! reload_completed && MEM_P (SUBREG_REG (op)))
- return general_operand (op, mode);
- op = SUBREG_REG (op);
- }
-
- return (REG_P (op)
- && (REGNO (op) >= FIRST_PSEUDO_REGISTER
- || in_hard_reg_set_p (operand_reg_set,
- GET_MODE (op), REGNO (op))));
+ return register_operand (op, mode);
}
/* Return 1 if OP is a valid operand that stands for pushing a
