This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][AArch64] print_operand should not fallthrough from register operand into generic operand
- From: James Greenhalgh <james dot greenhalgh at arm dot com>
- To: Wilco Dijkstra <Wilco dot Dijkstra at arm dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, nd <nd at arm dot com>
- Date: Wed, 27 Apr 2016 16:13:39 +0100
- Subject: Re: [PATCH][AArch64] print_operand should not fallthrough from register operand into generic operand
- Authentication-results: sourceware.org; auth=none
- Nodisclaimer: True
- References: <AM3PR08MB0088DADEA1075248CDE32E20836F0 at AM3PR08MB0088 dot eurprd08 dot prod dot outlook dot com>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:23
On Fri, Apr 22, 2016 at 02:24:49PM +0000, Wilco Dijkstra wrote:
> Some patterns are using '%w2' for immediate operands, which means that a zero
> immediate is actually emitted as 'wzr' or 'xzr'. This not only changes an
> immediate operand into a register operand but may emit illegal instructions
> from legal RTL (eg. ORR x0, SP, xzr rather than ORR x0, SP, 0).
>
> Remove the fallthrough in aarch64_print_operand from the 'w' and 'x' case
> into the '0' case that created this issue. Modify a few patterns to use '%2'
> rather than '%w2' for an immediate or memory operand so they now print
> correctly without the fallthrough.
>
> OK for trunk?
>
> (note this requires https://gcc.gnu.org/ml/gcc-patches/2016-04/msg01265.html to
> be committed first)
If you've got dependencies like this, formatting the mails as a patch set
makes review easier. e.g.:
[PATCH 1/2 AArch64] Fix shift attributes
[PATCH 2/2 AArch64] print_operand should not fallthrough from register
operand into generic operand
My biggest concern about this patch is that it might break code that is in
the wild. Looks to me like this breaks (at least)
arch/arm64/asm/percpu.h in the kernel.
So the part of this patch removing the fallthrough to general operand
is not OK for trunk.
The other parts look reasonable to me, please resubmit just those.
> --
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 881dc52e2de03231abb217a9ce22cbb1cc44bc6c..bcef50825c8315c39e29dbe57c387ea2a4fe445d 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -4608,7 +4608,8 @@ aarch64_print_operand (FILE *f, rtx x, int code)
> break;
> }
>
> - /* Fall through */
> + output_operand_lossage ("invalid operand for '%%%c'", code);
> + return;
>
> case 0:
> /* Print a normal operand, if it's a general register, then we
To be clear, this is the hunk that is not OK.
Thanks,
James