recog/reload: Remove old UNARY_P operand support
Checks
Commit Message
reload and constrain_operands had some old code to look through unary
operators. E.g. an operand could be (sign_extend (reg X)), and the
constraints would match the reg rather than the sign_extend.
This was previously used by the MIPS port. But relying on it was a
recurring source of problems, so Eric and I removed it in the MIPS
rewrite from ~20 years back. I don't know of any other port that used it.
Also, the constraints processing in LRA and IRA do not have direct
support for these embedded operators, so I think it was only ever a
reload-specific feature (and probably only a global/local+reload-specific
feature, rather than IRA+reload).
Keeping the checks caused problems for special memory constraints,
leading to:
/* A unary operator may be accepted by the predicate, but it
is irrelevant for matching constraints. */
/* For special_memory_operand, there could be a memory operand inside,
and it would cause a mismatch for constraint_satisfied_p. */
if (UNARY_P (op) && op == extract_mem_from_operand (op))
op = XEXP (op, 0);
But inline asms are another source of problems. Asms don't have
predicates, and so we can't use recog to decide whether a given change
to an asm gives a valid match. We instead rely on constrain_operands as
something of a recog stand-in. For an example like:
void
foo (int *ptr)
{
asm volatile ("%0" :: "r" (-*ptr));
}
any attempt to propagate the negation into the asm would be allowed,
because it's the negated register that would be checked against the
"r" constraint. This would later lead to:
error: invalid 'asm': invalid operand
The same thing happened in gcc.target/aarch64/vneg_s.c with the
upcoming late-combine pass.
Rather than add more workarounds, it seemed better just to delete
this code.
Tested on aarch64-linux-gnu & x86_64-linux-gnu. OK to install?
Richard
gcc/
* recog.cc (constrain_operands): Remove UNARY_P handling.
* reload.cc (find_reloads): Likewise.
---
gcc/recog.cc | 15 ---------------
gcc/reload.cc | 6 ------
2 files changed, 21 deletions(-)
Comments
On 10/24/23 04:14, Richard Sandiford wrote:
> reload and constrain_operands had some old code to look through unary
> operators. E.g. an operand could be (sign_extend (reg X)), and the
> constraints would match the reg rather than the sign_extend. >
> This was previously used by the MIPS port. But relying on it was a
> recurring source of problems, so Eric and I removed it in the MIPS
> rewrite from ~20 years back. I don't know of any other port that used it.
I can't remember if other ports used this or not. The most likely
scenario would be a port from the mid/late 90s that started as a 32bit
port and was extended to a 64bit port and has similar sign extension
properties as MIPS.
PPC, sparc and s390 come immediately to mind. I just checked their
predicates.md files and they don't see to have a predicate which would
trigger this old code, even if they were reload targets.
>
> Also, the constraints processing in LRA and IRA do not have direct
> support for these embedded operators, so I think it was only ever a
> reload-specific feature (and probably only a global/local+reload-specific
> feature, rather than IRA+reload).
It was definitely specific to the old register allocator+reload
implementation. It pre-dates the introduction of IRA by many years.
>
> Richard
>
>
> gcc/
> * recog.cc (constrain_operands): Remove
OK
jeff
> From: Richard Sandiford <richard.sandiford@arm.com>
> Date: Tue, 24 Oct 2023 11:14:20 +0100
> reload and constrain_operands had some old code to look through unary
> operators. E.g. an operand could be (sign_extend (reg X)), and the
> constraints would match the reg rather than the sign_extend.
>
> This was previously used by the MIPS port. But relying on it was a
> recurring source of problems, so Eric and I removed it in the MIPS
> rewrite from ~20 years back. I don't know of any other port that used it.
The SH did. I remember this being one of the ugliest warts
of reload. IIRC, there was a bit of a discourse involving
me and Joern way-back (also IIRC some 20 years ago, at least
before IRA and LRA). The conclusion was that removing this
misfeature would be ok, as already at that time, there was
no de-facto beneficial effect for sh, likely due to code
rot. However, no action was taken; no code changed.
Thanks for removing the last(?) bits!
brgds, H-P
@@ -3080,13 +3080,6 @@ constrain_operands (int strict, alternative_mask alternatives)
earlyclobber[opno] = 0;
- /* A unary operator may be accepted by the predicate, but it
- is irrelevant for matching constraints. */
- /* For special_memory_operand, there could be a memory operand inside,
- and it would cause a mismatch for constraint_satisfied_p. */
- if (UNARY_P (op) && op == extract_mem_from_operand (op))
- op = XEXP (op, 0);
-
if (GET_CODE (op) == SUBREG)
{
if (REG_P (SUBREG_REG (op))
@@ -3152,14 +3145,6 @@ constrain_operands (int strict, alternative_mask alternatives)
{
rtx op1 = recog_data.operand[match];
rtx op2 = recog_data.operand[opno];
-
- /* A unary operator may be accepted by the predicate,
- but it is irrelevant for matching constraints. */
- if (UNARY_P (op1))
- op1 = XEXP (op1, 0);
- if (UNARY_P (op2))
- op2 = XEXP (op2, 0);
-
val = operands_match_p (op1, op2);
}
@@ -3077,12 +3077,6 @@ find_reloads (rtx_insn *insn, int replace, int ind_levels, int live_known,
enum constraint_num cn;
enum reg_class cl;
- /* If the predicate accepts a unary operator, it means that
- we need to reload the operand, but do not do this for
- match_operator and friends. */
- if (UNARY_P (operand) && *p != 0)
- operand = XEXP (operand, 0);
-
/* If the operand is a SUBREG, extract
the REG or MEM (or maybe even a constant) within.
(Constants can occur as a result of reg_equiv_constant.) */