Re: [4.7][SH] Binary compatibility with atomic_test_and_test_trueval != 1
On 03/04/2012 11:09 AM, Oleg Endo wrote: Richard, could you also please take the TARGET_ATOMIC_TEST_AND_SET_TRUEVAL hunk from this patch for the 4.7 branch? Done. I've also moved the TARGET_ATOMIC_TEST_AND_SET_TRUEVAL definition from sh.h to sh.c where it belongs. r~
Re: [4.7][SH] Binary compatibility with atomic_test_and_test_trueval != 1
On Mon, 2012-03-05 at 11:00 -0800, Richard Henderson wrote: On 03/04/2012 11:09 AM, Oleg Endo wrote: Richard, could you also please take the TARGET_ATOMIC_TEST_AND_SET_TRUEVAL hunk from this patch for the 4.7 branch? Done. Thanks! I've also moved the TARGET_ATOMIC_TEST_AND_SET_TRUEVAL definition from sh.h to sh.c where it belongs. Yeah, however, I'm also using the value behind TARGET_ATOMIC_TEST_AND_SET_TRUEVAL in sync.md. If it's in sh.c it doesn't work. That's why I left it in sh.h. Cheers, Oleg
Re: [4.7][SH] Binary compatibility with atomic_test_and_test_trueval != 1
On 03/05/2012 01:44 PM, Oleg Endo wrote: Yeah, however, I'm also using the value behind TARGET_ATOMIC_TEST_AND_SET_TRUEVAL in sync.md. If it's in sh.c it doesn't work. That's why I left it in sh.h. That value should be available via targetm.atomic_test_and_set_trueval. r~
Re: [4.7][SH] Binary compatibility with atomic_test_and_test_trueval != 1
On 03/05/2012 01:49 PM, Richard Henderson wrote:
On 03/05/2012 01:44 PM, Oleg Endo wrote:
Yeah, however, I'm also using the value behind
TARGET_ATOMIC_TEST_AND_SET_TRUEVAL in sync.md. If it's in sh.c it
doesn't work. That's why I left it in sh.h.
That value should be available via targetm.atomic_test_and_set_trueval.
Fixed thus.
r~
diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 9a35295..0b7e635 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -3495,8 +3495,8 @@ insn-automata.o : insn-automata.c $(CONFIG_H) $(SYSTEM_H)
coretypes.h \
insn-emit.o : insn-emit.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \
$(RTL_H) $(TM_P_H) $(FUNCTION_H) $(EXPR_H) $(OPTABS_H) \
dfp.h $(FLAGS_H) output.h insn-config.h hard-reg-set.h $(RECOG_H)\
- $(RESOURCE_H) reload.h $(DIAGNOSTIC_CORE_H) $(REGS_H) tm-constrs.h $(GGC_H)
\
- $(BASIC_BLOCK_H) $(INTEGRATE_H)
+ $(RESOURCE_H) reload.h $(DIAGNOSTIC_CORE_H) $(REGS_H) tm-constrs.h \
+ $(GGC_H) $(BASIC_BLOCK_H) $(INTEGRATE_H) $(TARGET_H)
insn-enums.o : insn-enums.c $(CONFIG_H) $(SYSTEM_H) insn-constants.h
insn-extract.o : insn-extract.c $(CONFIG_H) $(SYSTEM_H) coretypes.h\
$(TM_H) $(RTL_H) $(DIAGNOSTIC_CORE_H) insn-config.h $(RECOG_H)
diff --git a/gcc/config/sh/sync.md b/gcc/config/sh/sync.md
index 113288c..258e048 100644
--- a/gcc/config/sh/sync.md
+++ b/gcc/config/sh/sync.md
@@ -417,9 +417,10 @@
emit_insn (gen_tasb (addr));
else
{
- rtx val = force_reg (QImode,
- gen_int_mode (TARGET_ATOMIC_TEST_AND_SET_TRUEVAL,
-QImode));
+ rtx val;
+
+ val = gen_int_mode (targetm.atomic_test_and_set_trueval, QImode);
+ val = force_reg (QImode, val);
emit_insn (gen_atomic_test_and_set_soft (addr, val));
}
diff --git a/gcc/genemit.c b/gcc/genemit.c
index 662d8ca..173e4d3 100644
--- a/gcc/genemit.c
+++ b/gcc/genemit.c
@@ -812,7 +812,8 @@ from the machine description file `md'. */\n\n);
printf (#include \tm-constrs.h\\n);
printf (#include \ggc.h\\n);
printf (#include \basic-block.h\\n);
- printf (#include \integrate.h\\n\n);
+ printf (#include \integrate.h\\n);
+ printf (#include \target.h\\n\n);
printf (#define FAIL return (end_sequence (), _val)\n);
printf (#define DONE return (_val = get_insns (), end_sequence (),
_val)\n\n);
Re: [4.7][SH] Binary compatibility with atomic_test_and_test_trueval != 1
On Sat, 2012-03-03 at 10:31 -0800, Richard Henderson wrote:
On 03/02/2012 10:11 AM, Richard Henderson wrote:
I'm in the process of sanity testing this on x86_64 with trueval set to
0x80.
Jakub, ok for 4.7 branch if it passes?
* optabs.c (expand_atomic_test_and_set): Honor
atomic_test_and_set_trueval even when atomic_test_and_set
optab is not in use.
I've committed this patch to mainline. I still think it ought to
go onto the 4.7 branch...
Attached is a slightly modified version of the patch from
http://gcc.gnu.org/ml/gcc-patches/2012-03/msg00085.html
I have removed the signed char weirdo and adjusted the comment above
TARGET_ATOMIC_TEST_AND_SET_TRUEVAL accordingly.
Tested by compiling some test functions that use __atomic_test_and_set /
__GCC_ATOMIC_TEST_AND_SET_TRUEVAL with various SH atomic option
combinations and looking at the output asm.
OK to apply to trunk?
Richard, could you also please take the
TARGET_ATOMIC_TEST_AND_SET_TRUEVAL hunk from this patch for the 4.7
branch?
Cheers,
Oleg
2012-03-04 Oleg Endo olege...@gcc.gnu.org
* config/sh/sh.h (TARGET_ATOMIC_TEST_AND_SET_TRUEVAL): New hook.
* config/sh/sync.md (atomic_test_and_set): New expander.
(tasb, atomic_test_and_set_soft): New insns.
* config/sh/sh.opt (menable-tas): New option.
* doc/invoke.texi (SH Options): Document it.
Index: gcc/doc/invoke.texi
===
--- gcc/doc/invoke.texi (revision 184877)
+++ gcc/doc/invoke.texi (working copy)
@@ -887,7 +887,8 @@
-mdivsi3_libfunc=@var{name} -mfixed-range=@var{register-range} @gol
-madjust-unroll -mindexed-addressing -mgettrcost=@var{number} -mpt-fixed @gol
-maccumulate-outgoing-args -minvalid-symbols -msoft-atomic @gol
--mbranch-cost=@var{num} -mcbranchdi -mcmpeqdi -mfused-madd -mpretend-cmove}
+-mbranch-cost=@var{num} -mcbranchdi -mcmpeqdi -mfused-madd -mpretend-cmove @gol
+-menable-tas}
@emph{Solaris 2 Options}
@gccoptlist{-mimpure-text -mno-impure-text @gol
@@ -17823,6 +17824,15 @@
This option is enabled by default when the target is @code{sh-*-linux*}.
For details on the atomic built-in functions see @ref{__atomic Builtins}.
+@item -menable-tas
+@opindex menable-tas
+Generate the @code{tas.b} opcode for @code{__atomic_test_and_set}.
+Notice that depending on the particular hardware and software configuration
+this can degrade overall performance due to the operand cache line flushes
+that are implied by the @code{tas.b} instruction. On multi-core SH4A
+processors the @code{tas.b} instruction must be used with caution since it
+can result in data corruption for certain cache configurations.
+
@item -mspace
@opindex mspace
Optimize for space instead of speed. Implied by @option{-Os}.
Index: gcc/config/sh/sh.h
===
--- gcc/config/sh/sh.h (revision 184877)
+++ gcc/config/sh/sh.h (working copy)
@@ -2473,4 +2473,10 @@
/* FIXME: middle-end support for highpart optimizations is missing. */
#define high_life_started reload_in_progress
+/* The tas.b instruction sets the 7th bit in the byte, i.e. 0x80.
+ This value is used by optabs.c atomic op expansion code as well as in
+ sync.md. */
+#undef TARGET_ATOMIC_TEST_AND_SET_TRUEVAL
+#define TARGET_ATOMIC_TEST_AND_SET_TRUEVAL 0x80
+
#endif /* ! GCC_SH_H */
Index: gcc/config/sh/sync.md
===
--- gcc/config/sh/sync.md (revision 184877)
+++ gcc/config/sh/sync.md (working copy)
@@ -404,3 +404,61 @@
1: mov r1,r15;
}
[(set_attr length 18)])
+
+(define_expand atomic_test_and_set
+ [(match_operand:SI 0 register_operand ) ;; bool result output
+ (match_operand:QI 1 memory_operand ) ;; memory
+ (match_operand:SI 2 const_int_operand )] ;; model
+ (TARGET_SOFT_ATOMIC || TARGET_ENABLE_TAS) !TARGET_SHMEDIA
+{
+ rtx addr = force_reg (Pmode, XEXP (operands[1], 0));
+
+ if (TARGET_ENABLE_TAS)
+emit_insn (gen_tasb (addr));
+ else
+{
+ rtx val = force_reg (QImode,
+ gen_int_mode (TARGET_ATOMIC_TEST_AND_SET_TRUEVAL,
+ QImode));
+ emit_insn (gen_atomic_test_and_set_soft (addr, val));
+}
+
+ /* The result of the test op is the inverse of what we are
+ supposed to return. Thus invert the T bit. The inversion will be
+ potentially optimized away and integrated into surrounding code. */
+ emit_insn (gen_movnegt (operands[0]));
+ DONE;
+})
+
+(define_insn tasb
+ [(set (reg:SI T_REG)
+ (eq:SI (mem:QI (match_operand:SI 0 register_operand r))
+ (const_int 0)))
+ (set (mem:QI (match_dup 0))
+ (unspec:QI [(const_int 128)] UNSPEC_ATOMIC))]
+ TARGET_ENABLE_TAS !TARGET_SHMEDIA
+ tas.b @%0
+ [(set_attr insn_class co_group)])
+
+(define_insn atomic_test_and_set_soft
+ [(set (reg:SI T_REG)
+ (eq:SI (mem:QI (match_operand:SI 0 register_operand u))
+ (const_int 0)))
+ (set (mem:QI (match_dup 0))
+ (unspec:QI
Re: [4.7][SH] Binary compatibility with atomic_test_and_test_trueval != 1
Oleg Endo oleg.e...@t-online.de wrote: Attached is a slightly modified version of the patch from http://gcc.gnu.org/ml/gcc-patches/2012-03/msg00085.html I have removed the signed char weirdo and adjusted the comment above TARGET_ATOMIC_TEST_AND_SET_TRUEVAL accordingly. Tested by compiling some test functions that use __atomic_test_and_set / __GCC_ATOMIC_TEST_AND_SET_TRUEVAL with various SH atomic option combinations and looking at the output asm. OK to apply to trunk? OK. Regards, kaz
Re: [4.7][SH] Binary compatibility with atomic_test_and_test_trueval != 1
On 03/02/2012 10:11 AM, Richard Henderson wrote: I'm in the process of sanity testing this on x86_64 with trueval set to 0x80. Jakub, ok for 4.7 branch if it passes? * optabs.c (expand_atomic_test_and_set): Honor atomic_test_and_set_trueval even when atomic_test_and_set optab is not in use. I've committed this patch to mainline. I still think it ought to go onto the 4.7 branch... r~
Re: [4.7][SH] Binary compatibility with atomic_test_and_test_trueval != 1
On Sat, Mar 03, 2012 at 10:31:17AM -0800, Richard Henderson wrote: On 03/02/2012 10:11 AM, Richard Henderson wrote: I'm in the process of sanity testing this on x86_64 with trueval set to 0x80. Jakub, ok for 4.7 branch if it passes? * optabs.c (expand_atomic_test_and_set): Honor atomic_test_and_set_trueval even when atomic_test_and_set optab is not in use. I've committed this patch to mainline. I still think it ought to go onto the 4.7 branch... Ok. Jakub
[4.7][SH] Binary compatibility with atomic_test_and_test_trueval != 1
On 02/28/2012 07:16 AM, Oleg Endo wrote:
Wouldn't it make sense to use the value behind
TARGET_ATOMIC_TEST_AND_SET_TRUEVAL in optabs.c
(expand_atomic_test_and_set) instead of const1_rtx when emitting
generated atomic_exchange / atomic_compare_and_swap_exchange_loop?
Maybe something like the attached patch?
Background:
I'm working on a patch to add a new option -menable-tas (independent of
the existing -msoft-atomic option) which would allow the compiler to
generate SH's tas.b insn. This would allow using the tas.b insn without
the other atomic sequences, or in combination with them. The reason
behind this is that the tas.b insn might not always be appropriate to
use, depending on the particular system hardware/software setup (e.g.
dual-core SH4A).
On SH the TARGET_ATOMIC_TEST_AND_SET_TRUEVAL has to be defined as 0x80.
Having the generated atomic_compare_and_swap / atomic_exchange sequences
using 0x01 as the 'set' value might lead to inconsistencies when mixing
code that uses the tas.b insn and code that doesn't use it, which should
actually be OK to do.
That's a good idea.
For proper binary compatibility, we should probably introduce that asap.
You forgot two things in your patch, gen_int_mode and the fact that we
still have to return a boolean (0/1) value.
Also in order for the binary compatibility to work right, you'd want to
have the SH test-and-set-trueval set appropriately asap. Kaz, I assume
you'd agree that 0x80 is a good value for the tas.b insn? We don't
necessarily need to support tas.b right away, but getting trueval set
right is imperative.
I'm in the process of sanity testing this on x86_64 with trueval set to 0x80.
Jakub, ok for 4.7 branch if it passes?
r~
* optabs.c (expand_atomic_test_and_set): Honor
atomic_test_and_set_trueval even when atomic_test_and_set
optab is not in use.
diff --git a/gcc/optabs.c b/gcc/optabs.c
index b0ecdf0..fd353d7 100644
--- a/gcc/optabs.c
+++ b/gcc/optabs.c
@@ -7384,34 +7384,57 @@ rtx
expand_atomic_test_and_set (rtx target, rtx mem, enum memmodel model)
{
enum machine_mode mode = GET_MODE (mem);
- rtx ret;
+ rtx ret, trueval, subtarget;
ret = maybe_emit_atomic_test_and_set (target, mem, model);
if (ret)
return ret;
- if (target == NULL_RTX)
-target = gen_reg_rtx (mode);
+ /* Be binary compatible with non-default settings of trueval, and different
+ cpu revisions. E.g. one revision may have atomic-test-and-set, but
+ another only has atomic-exchange. */
+ if (targetm.atomic_test_and_set_trueval == 1)
+{
+ trueval = const1_rtx;
+ subtarget = target ? target : gen_reg_rtx (mode);
+}
+ else
+{
+ trueval = gen_int_mode (targetm.atomic_test_and_set_trueval, mode);
+ subtarget = gen_reg_rtx (mode);
+}
- /* If there is no test and set, try exchange, then a compare_and_swap loop,
- then __sync_test_and_set. */
- ret = maybe_emit_atomic_exchange (target, mem, const1_rtx, model);
- if (ret)
-return ret;
+ /* Try the atomic-exchange optab... */
+ ret = maybe_emit_atomic_exchange (subtarget, mem, trueval, model);
- ret = maybe_emit_compare_and_swap_exchange_loop (target, mem, const1_rtx);
- if (ret)
-return ret;
+ /* ... then an atomic-compare-and-swap loop ... */
+ if (!ret)
+ret = maybe_emit_compare_and_swap_exchange_loop (subtarget, mem, trueval);
- ret = maybe_emit_sync_lock_test_and_set (target, mem, const1_rtx, model);
- if (ret)
-return ret;
+ /* ... before trying the vaguely defined legacy lock_test_and_set. */
+ if (!ret)
+ret = maybe_emit_sync_lock_test_and_set (subtarget, mem, trueval, model);
- /* Failing all else, assume a single threaded environment and simply perform
- the operation. */
- emit_move_insn (target, mem);
- emit_move_insn (mem, const1_rtx);
- return target;
+ /* Recall that the legacy lock_test_and_set optab was allowed to do magic
+ things with the value 1. Thus we try again without trueval. */
+ if (!ret targetm.atomic_test_and_set_trueval != 1)
+ret = maybe_emit_sync_lock_test_and_set (subtarget, mem, const1_rtx,
model);
+
+ /* Failing all else, assume a single threaded environment and simply
+ perform the operation. */
+ if (!ret)
+{
+ emit_move_insn (subtarget, mem);
+ emit_move_insn (mem, trueval);
+ ret = subtarget;
+}
+
+ /* Recall that have to return a boolean value; rectify if trueval
+ is not exactly one. */
+ if (targetm.atomic_test_and_set_trueval != 1)
+ret = emit_store_flag_force (target, NE, ret, const0_rtx, mode, 0, 1);
+
+ return ret;
}
/* This function expands the atomic exchange operation:
Re: [4.7][SH] Binary compatibility with atomic_test_and_test_trueval != 1
On Fri, 2012-03-02 at 10:11 -0800, Richard Henderson wrote: For proper binary compatibility, we should probably introduce that asap. You forgot two things in your patch, gen_int_mode and the fact that we still have to return a boolean (0/1) value. Ah, yes, of course! Also in order for the binary compatibility to work right, you'd want to have the SH test-and-set-trueval set appropriately asap. Kaz, I assume you'd agree that 0x80 is a good value for the tas.b insn? We don't necessarily need to support tas.b right away, but getting trueval set right is imperative. I'm in the process of sanity testing this on x86_64 with trueval set to 0x80. Jakub, ok for 4.7 branch if it passes? Since we're now back to stage 1 again, I'm about to commit this one ... http://gcc.gnu.org/ml/gcc-patches/2012-03/msg00085.html So the SH tas.b instruction support should be there from 4.8. Attached is the hunk from the tas.b patch for 4.8 to define TARGET_ATOMIC_TEST_AND_SET_TRUEVAL. Cheers, Oleg Index: gcc/config/sh/sh.h === --- gcc/config/sh/sh.h (revision 184669) +++ gcc/config/sh/sh.h (working copy) @@ -2475,4 +2475,11 @@ /* FIXME: middle-end support for highpart optimizations is missing. */ #define high_life_started reload_in_progress +/* The tas.b instruction sets the 7th bit in the byte, i.e. 0x80. + This value is used by optabs.c atomic op expansion code as well as in + sync.md. It must be defined as signed char here or else the movqi + pattern will refuse to load it as a QImode constant. */ +#undef TARGET_ATOMIC_TEST_AND_SET_TRUEVAL +#define TARGET_ATOMIC_TEST_AND_SET_TRUEVAL ((signed char)0x80) + #endif /* ! GCC_SH_H */
Re: [4.7][SH] Binary compatibility with atomic_test_and_test_trueval != 1
On 03/02/2012 12:27 PM, Oleg Endo wrote: + This value is used by optabs.c atomic op expansion code as well as in + sync.md. It must be defined as signed char here or else the movqi + pattern will refuse to load it as a QImode constant. */ +#undef TARGET_ATOMIC_TEST_AND_SET_TRUEVAL +#define TARGET_ATOMIC_TEST_AND_SET_TRUEVAL ((signed char)0x80) The fix to use gen_mode_int obviates the need for the (signed char) hack. r~
Re: [4.7][SH] Binary compatibility with atomic_test_and_test_trueval != 1
On Fri, 2012-03-02 at 12:34 -0800, Richard Henderson wrote: On 03/02/2012 12:27 PM, Oleg Endo wrote: + This value is used by optabs.c atomic op expansion code as well as in + sync.md. It must be defined as signed char here or else the movqi + pattern will refuse to load it as a QImode constant. */ +#undef TARGET_ATOMIC_TEST_AND_SET_TRUEVAL +#define TARGET_ATOMIC_TEST_AND_SET_TRUEVAL ((signed char)0x80) The fix to use gen_mode_int obviates the need for the (signed char) hack. Ah right, sorry again. Then I'll rather wait with the whole tas.b patch until the optabs changes are in. Cheers, Oleg
Re: [4.7][SH] Binary compatibility with atomic_test_and_test_trueval != 1
Richard Henderson r...@redhat.com wrote: For proper binary compatibility, we should probably introduce that asap. You forgot two things in your patch, gen_int_mode and the fact that we still have to return a boolean (0/1) value. Also in order for the binary compatibility to work right, you'd want to have the SH test-and-set-trueval set appropriately asap. Kaz, I assume you'd agree that 0x80 is a good value for the tas.b insn? We don't necessarily need to support tas.b right away, but getting trueval set right is imperative. Yes, 0x80 is an appropriate value as you and oleg have suggested. Regards, kaz
