Message ID | 92B71FDE-F4D5-43D6-9BAF-23D5DC02850A@caviumnetworks.com |
---|---|
State | New |
Headers | show |
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
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"