[AArch64] Refactor acquire/release determination into output template

There is duplicate code for determining whether a load or store 
instruction needs acquire or release semantics.  This patch removes the duplicated code and uses a modifying operator to output a/l instead.  Since the testsuite already contains tests for the atomic functions, no new testcases are needed.

OK? Built and tested for aarch64-elf using Cavium's internal simulator with no regressions.

Thanks,
Joel Jones

ChangeLog:
   * config/aarch64/aarch64.c (aarch64_print_operand):  Add 'Q' and 'R' operator modifiers.
   * config/aarch64/atomics.md (atomic_load<mode>): Use 'Q' instead of returning a different template for acquire.
​    (aarch64_load_exclusive): ​Likewise.
   (atomic_store<mode>): Use 'R' instead of returning a different template for release.
   (aarch64_store_exclusive): Likewise.

On Tue, Jun 3, 2014 at 5:07 PM, Jones, Joel
<Joel.Jones@caviumnetworks.com> wrote:
> There is duplicate code for determining whether a load or store
> instruction needs acquire or release semantics.  This patch removes the duplicated code and uses a modifying operator to output a/l instead.  Since the testsuite already contains tests for the atomic functions, no new testcases are needed.
>
> OK? Built and tested for aarch64-elf using Cavium's internal simulator with no regressions.

Ping?


>
> Thanks,
> Joel Jones
>
> ChangeLog:
>    * config/aarch64/aarch64.c (aarch64_print_operand):  Add 'Q' and 'R' operator modifiers.
>    * config/aarch64/atomics.md (atomic_load<mode>): Use 'Q' instead of returning a different template for acquire.
>    (aarch64_load_exclusive): Likewise.
>    (atomic_store<mode>): Use 'R' instead of returning a different template for release.
>    (aarch64_store_exclusive): Likewise.
>
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index c2f6c4f..56152a0 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -3931,6 +3931,50 @@ aarch64_print_operand (FILE *f, rtx x, char code)
>        output_addr_const (asm_out_file, x);
>        break;
>
> +    case 'Q':
> +      {
> +        /* Print "a" if memory model requires ac'Q'uire semantics */
> +        if (GET_CODE (x) != CONST_INT)
> +          {
> +            output_operand_lossage ("invalid operand for '%%%c'", code);
> +            return;
> +          }
> +        enum memmodel model = (enum memmodel) INTVAL (x);
> +        bool is_acq = false;
> +        switch (model)
> +          {
> +            default: is_acq = true; break;
> +            case MEMMODEL_RELAXED:
> +            case MEMMODEL_CONSUME:
> +            case MEMMODEL_RELEASE: break;
> +          }
> +        if (is_acq)
> +          fputc ('a', f);
> +      }
> +      break;
> +
> +    case 'R':
> +      {
> +        /* Print "l" if memory model requires 'R'elease semantics */
> +        if (GET_CODE (x) != CONST_INT)
> +          {
> +            output_operand_lossage ("invalid operand for '%%%c'", code);
> +            return;
> +          }
> +        enum memmodel model = (enum memmodel) INTVAL (x);
> +        bool is_rel = false;
> +        switch (model)
> +          {
> +            default: is_rel = true; break;
> +            case MEMMODEL_RELAXED:
> +            case MEMMODEL_CONSUME:
> +            case MEMMODEL_ACQUIRE: break;
> +          }
> +        if (is_rel)
> +          fputc ('l', f);
> +      }
> +      break;
> +
>      default:
>        output_operand_lossage ("invalid operand prefix '%%%c'", code);
>        return;
> diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md
> index bffa465..55ba918 100644
> --- a/gcc/config/aarch64/atomics.md
> +++ b/gcc/config/aarch64/atomics.md
> @@ -259,15 +259,7 @@
>         (match_operand:SI 2 "const_int_operand")]                       ;; model
>        UNSPECV_LDA))]
>    ""
> -  {
> -    enum memmodel model = (enum memmodel) INTVAL (operands[2]);
> -    if (model == MEMMODEL_RELAXED
> -       || model == MEMMODEL_CONSUME
> -       || model == MEMMODEL_RELEASE)
> -      return "ldr<atomic_sfx>\t%<w>0, %1";
> -    else
> -      return "ldar<atomic_sfx>\t%<w>0, %1";
> -  }
> +  "ld%Q2r<atomic_sfx>\t%<w>0, %1"
>  )
>
>  (define_insn "atomic_store<mode>"
> @@ -277,15 +269,7 @@
>         (match_operand:SI 2 "const_int_operand")]                       ;; model
>        UNSPECV_STL))]
>    ""
> -  {
> -    enum memmodel model = (enum memmodel) INTVAL (operands[2]);
> -    if (model == MEMMODEL_RELAXED
> -       || model == MEMMODEL_CONSUME
> -       || model == MEMMODEL_ACQUIRE)
> -      return "str<atomic_sfx>\t%<w>1, %0";
> -    else
> -      return "stlr<atomic_sfx>\t%<w>1, %0";
> -  }
> +  "st%R2r<atomic_sfx>\t%<w>1, %0"
>  )
>
>  (define_insn "aarch64_load_exclusive<mode>"
> @@ -296,15 +280,7 @@
>          (match_operand:SI 2 "const_int_operand")]
>         UNSPECV_LX)))]
>    ""
> -  {
> -    enum memmodel model = (enum memmodel) INTVAL (operands[2]);
> -    if (model == MEMMODEL_RELAXED
> -       || model == MEMMODEL_CONSUME
> -       || model == MEMMODEL_RELEASE)
> -      return "ldxr<atomic_sfx>\t%w0, %1";
> -    else
> -      return "ldaxr<atomic_sfx>\t%w0, %1";
> -  }
> +  "ld%Q2xr<atomic_sfx>\t%w0, %1"
>  )
>
>  (define_insn "aarch64_load_exclusive<mode>"
> @@ -314,15 +290,7 @@
>         (match_operand:SI 2 "const_int_operand")]
>        UNSPECV_LX))]
>    ""
> -  {
> -    enum memmodel model = (enum memmodel) INTVAL (operands[2]);
> -    if (model == MEMMODEL_RELAXED
> -       || model == MEMMODEL_CONSUME
> -       || model == MEMMODEL_RELEASE)
> -      return "ldxr\t%<w>0, %1";
> -    else
> -      return "ldaxr\t%<w>0, %1";
> -  }
> +  "ld%Q2xr\t%<w>0, %1"
>  )
>
>  (define_insn "aarch64_store_exclusive<mode>"
> @@ -334,15 +302,7 @@
>         (match_operand:SI 3 "const_int_operand")]
>        UNSPECV_SX))]
>    ""
> -  {
> -    enum memmodel model = (enum memmodel) INTVAL (operands[3]);
> -    if (model == MEMMODEL_RELAXED
> -       || model == MEMMODEL_CONSUME
> -       || model == MEMMODEL_ACQUIRE)
> -      return "stxr<atomic_sfx>\t%w0, %<w>2, %1";
> -    else
> -      return "stlxr<atomic_sfx>\t%w0, %<w>2, %1";
> -  }
> +  "st%R3xr<atomic_sfx>\t%w0, %<w>2, %1";
>  )
>
>  (define_expand "mem_thread_fence"

On 4 June 2014 01:07, Jones, Joel <Joel.Jones@caviumnetworks.com> wrote:
> There is duplicate code for determining whether a load or store
> instruction needs acquire or release semantics.  This patch removes the duplicated code and uses a modifying operator to output a/l instead.  Since the testsuite already contains tests for the atomic functions, no new testcases are needed.
>
> OK? Built and tested for aarch64-elf using Cavium's internal simulator with no regressions.
>
> Thanks,
> Joel Jones


There are a limited number of single character output modifiers. I'd
rather not consume the available character space if it isn't
necessary, therefore I prefer not to restructure the code in the
manner proposed by this patch.

Cheers
/Marcus