On 7/15/24 08:33, Cupertino Miranda wrote:
> Both xchg and cmpxchg instructions, in the pseudo-C dialect, do not
> expect their memory address operand to be surrounded by parentheses.
> For example, it should be output as "w0 =cmpxchg32_32(r8+8,w0,w2)"
> instead of "w0 =cmpxchg32_32((r8+8),w0,w2)".
After some brief searching, looks like these extra parens in [cmp]xchg*
are an unintended consequence of:
bb5f619a938 bpf: fix printing of memory operands in pseudoc asm dialect
>
> This patch implements an operand modifier 'M' which marks the
> instruction templates that do not expect the parentheses, and adds it do
> xchg and cmpxchg templates.
One very minor nit below, otherwise LGTM. Please apply.
Thanks for the fix!
>
> gcc/ChangeLog:
> * config/bpf/atomic.md (atomic_compare_and_swap,
> atomic_exchange): Add operand modifier %M to the first
> operand.
> * config/bpf/bpf.cc (no_parentheses_mem_operand): Create
> variable.
> (bpf_print_operand): Set no_parentheses_mem_operand variable if
> %M operand is used.
> (bpf_print_operand_address): Conditionally output parentheses.
>
> gcc/testsuite/ChangeLog:
> * gcc.target/bpf/pseudoc-atomic-memaddr-op.c: Add test.
> ---
> gcc/config/bpf/atomic.md | 4 +--
> gcc/config/bpf/bpf.cc | 20 ++++++++---
> .../bpf/pseudoc-atomic-memaddr-op.c | 36 +++++++++++++++++++
> 3 files changed, 54 insertions(+), 6 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/bpf/pseudoc-atomic-memaddr-op.c
>
> diff --git a/gcc/config/bpf/atomic.md b/gcc/config/bpf/atomic.md
> index 65bd5f266a1..be4511bb51b 100644
> --- a/gcc/config/bpf/atomic.md
> +++ b/gcc/config/bpf/atomic.md
> @@ -129,7 +129,7 @@
> (set (match_dup 1)
> (match_operand:AMO 2 "nonmemory_operand" "0"))]
> "bpf_has_v3_atomics"
> - "{axchg<msuffix>\t%1,%0|%w0 = xchg<pcaxsuffix>(%1, %w0)}")
> + "{axchg<msuffix>\t%1,%0|%w0 = xchg<pcaxsuffix>(%M1, %w0)}")
>
> ;; The eBPF atomic-compare-and-exchange instruction has the form
> ;; acmp [%dst+offset], %src
> @@ -182,4 +182,4 @@
> (match_operand:AMO 3 "register_operand")] ;; desired
> UNSPEC_ACMP))]
> "bpf_has_v3_atomics"
> - "{acmp<msuffix>\t%1,%3|%w0 = cmpxchg<pcaxsuffix>(%1, %w0, %w3)}")
> + "{acmp<msuffix>\t%1,%3|%w0 = cmpxchg<pcaxsuffix>(%M1, %w0, %w3)}")
> diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc
> index 687e7ba49c1..a6b6e20731c 100644
> --- a/gcc/config/bpf/bpf.cc
> +++ b/gcc/config/bpf/bpf.cc
> @@ -835,6 +835,11 @@ bpf_print_register (FILE *file, rtx op, int code)
> }
> }
>
> +/* Variable defined to implement 'M' operand modifier for the special cases
> + where the parentheses should not be printed surrounding a memory address
> + operand. */
> +static bool no_parentheses_mem_operand;
> +
> /* Print an instruction operand. This function is called in the macro
> PRINT_OPERAND defined in bpf.h */
>
> @@ -847,6 +852,7 @@ bpf_print_operand (FILE *file, rtx op, int code)
> bpf_print_register (file, op, code);
> break;
> case MEM:
> + no_parentheses_mem_operand = (code == 'M');
> output_address (GET_MODE (op), XEXP (op, 0));
> break;
> case CONST_DOUBLE:
> @@ -891,6 +897,9 @@ bpf_print_operand (FILE *file, rtx op, int code)
> }
> }
>
> +#define PAREN_OPEN (asm_dialect == ASM_NORMAL ? "[" :
> no_parentheses_mem_operand ? "" : "(")
> +#define PAREN_CLOSE (asm_dialect == ASM_NORMAL ? "]" :
> no_parentheses_mem_operand ? "" : ")")
> +
> /* Print an operand which is an address. This function should handle
> any legit address, as accepted by bpf_legitimate_address_p, and
> also addresses that are valid in CALL instructions.
> @@ -904,9 +913,9 @@ bpf_print_operand_address (FILE *file, rtx addr)
> switch (GET_CODE (addr))
> {
> case REG:
> - fprintf (file, asm_dialect == ASM_NORMAL ? "[" : "(");
> + fprintf (file, "%s", PAREN_OPEN);
> bpf_print_register (file, addr, 0);
> - fprintf (file, asm_dialect == ASM_NORMAL ? "+0]" : "+0)");
> + fprintf (file, "+0%s", PAREN_CLOSE);
> break;
> case PLUS:
> {
> @@ -923,14 +932,14 @@ bpf_print_operand_address (FILE *file, rtx addr)
> || (GET_CODE (op1) == UNSPEC
> && XINT (op1, 1) == UNSPEC_CORE_RELOC)))
> {
> - fprintf (file, asm_dialect == ASM_NORMAL ? "[" : "(");
> + fprintf (file, "%s", PAREN_OPEN);
> bpf_print_register (file, op0, 0);
> fprintf (file, "+");
> if (GET_CODE (op1) == UNSPEC)
> output_addr_const (file, XVECEXP (op1, 0, 0));
> else
> output_addr_const (file, op1);
> - fprintf (file, asm_dialect == ASM_NORMAL ? "]" : ")");
> + fprintf (file, "%s", PAREN_CLOSE);
> }
> else
> fatal_insn ("invalid address in operand", addr);
> @@ -948,6 +957,9 @@ bpf_print_operand_address (FILE *file, rtx addr)
> }
> }
>
> +#undef PAREN_OPEN
> +#undef PAREN_CLOSE
> +
> /* Add a BPF builtin function with NAME, CODE and TYPE. Return
> the function decl or NULL_TREE if the builtin was not added. */
>
> diff --git a/gcc/testsuite/gcc.target/bpf/pseudoc-atomic-memaddr-op.c
> b/gcc/testsuite/gcc.target/bpf/pseudoc-atomic-memaddr-op.c
> new file mode 100644
> index 00000000000..758faf04cc2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/bpf/pseudoc-atomic-memaddr-op.c
> @@ -0,0 +1,36 @@
> +/* { dg-do compile } */
> +/* { dg-options "-mv3-atomics -O2 -masm=pseudoc" } */
> +
> +int
> +foo (int *p, int *expected, int desired)
> +{
> + return __atomic_compare_exchange (p, expected, &desired, 0,
> + __ATOMIC_ACQUIRE, __ATOMIC_RELAXED);
> +}
> +
> +int
> +foo64 (long *p, long *expected, long desired)
> +{
> + return __atomic_compare_exchange (p, expected, &desired, 0,
> + __ATOMIC_ACQUIRE, __ATOMIC_RELAXED);
> +}
> +
> +int bar (int *p, int *new)
nit: style - return type should be on its own line
> +{
> + int old;
> + __atomic_exchange (p, new, &old, __ATOMIC_RELAXED);
> + return old;
> +}
> +
> +int bar64 (long *p, long *new)
same here
> +{
> + long old;
> + __atomic_exchange (p, new, &old, __ATOMIC_SEQ_CST);
> + return old;
> +}
> +
> +/* { dg-final { scan-assembler "r. = cmpxchg_64\\(r.\\+0, r., r.\\)" } } */
> +/* { dg-final { scan-assembler "w. = cmpxchg32_32\\(r.\\+0, w., w.\\)" } } */
> +/* { dg-final { scan-assembler-times "w. = xchg32_32\\(r.\\+0, w.\\)" 1 } }
> */
> +/* { dg-final { scan-assembler-times "r. = xchg_64\\(r.\\+0, r.\\)" 1 } } */
> +
