diff mbox

[m68k,sparc] Fix atomic_test_and_set

Message ID 1330442171.2929.150.camel@yam-132-YW-E178-FTW
State New
Headers show

Commit Message

Oleg Endo Feb. 28, 2012, 3:16 p.m. UTC 
On Fri, 2012-01-27 at 09:29 +1100, Richard Henderson wrote:
> Two of the patches have been posted here before; the libstdc++
> patch was approved by Benjamin.
> 
> All of the patches tested on sparc64-linux, and sanity checked
> on x86_64-linux.  I've cross-compiled for m68k-linux, but I've
> only been able to visually sanity check the code in libstdc++.
> 
> Committed.  Hopefully that wraps up the atomic patches...
> 

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.

Cheers,
Oleg
diff mbox

Patch

Index: gcc/optabs.c
===================================================================
--- gcc/optabs.c	(revision 184589)
+++ gcc/optabs.c	(working copy)
@@ -7384,6 +7384,7 @@ 
 expand_atomic_test_and_set (rtx target, rtx mem, enum memmodel model)
 {
   enum machine_mode mode = GET_MODE (mem);
+  rtx tas_trueval = GEN_INT (targetm.atomic_test_and_set_trueval);
   rtx ret;
 
   ret = maybe_emit_atomic_test_and_set (target, mem, model);
@@ -7395,22 +7396,22 @@ 
 
   /* 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);
+  ret = maybe_emit_atomic_exchange (target, mem, tas_trueval, model);
   if (ret)
     return ret;
 
-  ret = maybe_emit_compare_and_swap_exchange_loop (target, mem, const1_rtx);
+  ret = maybe_emit_compare_and_swap_exchange_loop (target, mem, tas_trueval);
   if (ret)
     return ret;
 
-  ret = maybe_emit_sync_lock_test_and_set (target, mem, const1_rtx, model);
+  ret = maybe_emit_sync_lock_test_and_set (target, mem, tas_trueval, model);
   if (ret)
     return ret;
 
   /* Failing all else, assume a single threaded environment and simply perform
      the operation.  */
   emit_move_insn (target, mem);
-  emit_move_insn (mem, const1_rtx);
+  emit_move_insn (mem, tas_trueval);
   return target;
 }