This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] ARM: fix macro expansion with a pld insn as a macro argument
- From: "Jan Beulich" <JBeulich at suse dot com>
- To: "Roland McGrath" <mcgrathr at google dot com>
- Cc: <binutils at sourceware dot org>
- Date: Mon, 12 Nov 2012 07:45:21 +0000
- Subject: Re: [PATCH] ARM: fix macro expansion with a pld insn as a macro argument
- References: <CAB=4xhq4nBcjscHbhDKartureQcbHY32XXdzTnW+Xf1AJ=D-Bg@mail.gmail.com>
>>> On 09.11.12 at 23:09, Roland McGrath <mcgrathr@google.com> wrote:
> ARM operands can contain the characters '#', '[', and ']'.
> But APP doesn't know this. This breaks cases like the one
> in the test case below:
>
> .macro foo arg, rest:vararg
> \rest
> .endm
> foo r0, pld [r0]
>
> Here APP squashes the line to "foo r0,pld[r0]" before macro expansion.
> When it actually gets to md_assemble, it's "pld[r0]" and that won't fly.
>
> I have been using macros of this nature (where the instruction mnemonic
> appears as part of a macro argument) for some time. It just so happened
> all the instructions I'd used this way heretofore had been ones whose first
> operand standards with an alphanumeric character. Now I tried it using
> the "pld" instruction, whose first operand has the syntax of a ld* or st*
> instruction's second operand, starting with a '['.
>
> The fix seems quite straightforward and safe.
Including the possibility of symbol names now suddenly being
allowed to include those characters?
> No regressions in 'make check' for arm-nacl and arm-linux-gnueabi targets.
>
> Ok for trunk?
> Ok for 2.23 branch?
>
>
> Thanks,
> Roland
>
>
> gas/
> 2012-11-09 Roland McGrath <mcgrathr@google.com>
>
> * config/tc-arm.c (arm_symbol_chars): New variable.
> * config/tc-arm.h (tc_symbol_chars): New macro, defined to that.
>
> gas/testsuite/
> 2012-11-09 Roland McGrath <mcgrathr@google.com>
>
> * gas/arm/macro-pld.s: New file.
> * gas/arm/macro-pld.d: New file.
>
> --- a/gas/config/tc-arm.c
> +++ b/gas/config/tc-arm.c
> @@ -321,6 +321,11 @@ static int implicit_it_mode = IMPLICIT_IT_MODE_ARM;
>
> static bfd_boolean unified_syntax = FALSE;
>
> +/* An immediate operand can start with #, and ld*, st*, pld operands
> + can contain [ and ]. We need to tell APP not to elide whitespace
> + before a [, which can appear as the first operand for pld. */
> +const char arm_symbol_chars[] = "#[]";
For your purposes, wouldn't it suffice to just have "[" here? Or
otherwise, the comment appears inconsistent with the actual set
of characters.
Along with the first comment above, this seems the wrong
approach to me in any case. Instead, I would expect these
characters to be treated as operator ones, which ought to have
the effect of the elided white space to be benign. Or is there a
strict need to have white space between opcode and first
operand, even if the boundary is recognizable without?
Jan
> +
> enum neon_el_type
> {
> NT_invtype,
> --- a/gas/config/tc-arm.h
> +++ b/gas/config/tc-arm.h
> @@ -82,6 +82,9 @@ struct fix;
> /* We support double slash line-comments for compatibility with the
> ARM AArch64 Assembler. */
> #define DOUBLESLASH_LINE_COMMENTS
>
> +#define tc_symbol_chars arm_symbol_chars
> +extern const char arm_symbol_chars[];
> +
> #define TC_FORCE_RELOCATION(FIX) arm_force_relocation (FIX)
>
> extern unsigned int arm_frag_max_var (struct frag *);
> --- /dev/null
> +++ b/gas/testsuite/gas/arm/macro-pld.d
> @@ -0,0 +1,8 @@
> +#objdump: -dr
> +
> +.*: file format .*
> +
> +Disassembly of section \.text:
> +
> +0+ <.*>:
> +\s*0:\s+f5d0f000\s+pld\s+\[r0\]
> --- /dev/null
> +++ b/gas/testsuite/gas/arm/macro-pld.s
> @@ -0,0 +1,4 @@
> +.macro foo arg, rest:vararg
> + \rest
> +.endm
> + foo r0, pld [r0]