* [AArch64,PATCH] Refactor acquire/release determination into output template
@ 2014-06-04 0:07 Jones, Joel
2014-06-26 17:37 ` Andrew Pinski
2014-07-03 8:35 ` Marcus Shawcroft
0 siblings, 2 replies; 3+ messages in thread
From: Jones, Joel @ 2014-06-04 0:07 UTC (permalink / raw)
To: gcc-patches
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.
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"
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [AArch64,PATCH] Refactor acquire/release determination into output template
2014-06-04 0:07 [AArch64,PATCH] Refactor acquire/release determination into output template Jones, Joel
@ 2014-06-26 17:37 ` Andrew Pinski
2014-07-03 8:35 ` Marcus Shawcroft
1 sibling, 0 replies; 3+ messages in thread
From: Andrew Pinski @ 2014-06-26 17:37 UTC (permalink / raw)
To: Jones, Joel, Richard Earnshaw; +Cc: gcc-patches
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"
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [AArch64,PATCH] Refactor acquire/release determination into output template
2014-06-04 0:07 [AArch64,PATCH] Refactor acquire/release determination into output template Jones, Joel
2014-06-26 17:37 ` Andrew Pinski
@ 2014-07-03 8:35 ` Marcus Shawcroft
1 sibling, 0 replies; 3+ messages in thread
From: Marcus Shawcroft @ 2014-07-03 8:35 UTC (permalink / raw)
To: Jones, Joel; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-07-03 8:35 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-04 0:07 [AArch64,PATCH] Refactor acquire/release determination into output template Jones, Joel
2014-06-26 17:37 ` Andrew Pinski
2014-07-03 8:35 ` Marcus Shawcroft
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).