diff mbox

atomic test and set re-org.

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

Commit Message

Oleg Endo Dec. 21, 2011, 12:49 p.m. UTC 
On Thu, 2011-11-24 at 15:50 -0500, Andrew MacLeod wrote:
> This patch adds missing pattern support for atomic_test_and_set and 
> atomic_clear operations.   It also restructures the code for 
> atomic_test_and_set, atomic_exchange, and __sync_lock_test_and_set so 
> that it is easier to read and tries things in a rational manner.
> 
> bootstrapped on x86_64-unknown-linux-gnu with no new regressions.
> 
> Andrew

I've tried adding support for the new atomic_test_and_set to the SH
target and ran into problems when compiling code that would use the
return value of the atomic_test_and_set pattern.

The attached patch fixes this problem and also wraps the calls to
gen_atomic_test_and_set in the way it is done by other maybe_emit_*
functions.
Could you please have a look at it?

BTW, currently the only target utilizing the new atomic_test_and_set is
SPARC.  However, as far as I've observed an expander like

(define_expand "atomic_test_and_set<mode>"

will never be used, because currently the atomic_test_and_set never has
a mode in its name.  As far as I understand it, atomic_test_and_set is
supposed to operate on a bool only and thus no additional mode info is
needed.  Or is something else missing?

Cheers,
Oleg

2011-12-21  Oleg Endo  <oleg.endo@t-online.de>

	* optab.c (maybe_emit_atomic_test_and_set): New function.
	(expand_sync_lock_test_and_set): Use it.
	(expand_atomic_test_and_set): Use it.

Comments

Richard Henderson Dec. 21, 2011, 4:16 p.m. UTC | #1 
On 12/21/2011 04:49 AM, Oleg Endo wrote:
> BTW, currently the only target utilizing the new atomic_test_and_set is
> SPARC.  However, as far as I've observed an expander like
> 
> (define_expand "atomic_test_and_set<mode>"

That's a mistake.  The mode is significant -- it's the mode of the memory.
I'm sorry I missed that when reviewing Andrew's patch.

I guess there's more that'll be needed to actually make the code work.


r~
diff mbox

Patch

Index: gcc/optabs.c
===================================================================
--- gcc/optabs.c	(revision 182531)
+++ gcc/optabs.c	(working copy)
@@ -7436,10 +7436,35 @@ 
   return NULL_RTX;
 }
 
+/* This function tries to emit an atomic test and set operation using
+   __atomic_test_and_set, if it is defined in the target.  */
+
+static rtx
+maybe_emit_atomic_test_and_set (rtx target, rtx mem, enum memmodel model)
+{
 #ifndef HAVE_atomic_test_and_set
-#define HAVE_atomic_test_and_set 0
-#define gen_atomic_test_and_set(x,y,z)  (gcc_unreachable (), NULL_RTX)
+
+  return NULL_RTX;
+
+#else
+
+  /*  Allow the target to use a different mode than mem for storing
+      the previous value in some target specific way, as in
+      compare_and_swap.
+      This allows test and set conditional operations to be combined
+      better with surrounding code.  */
+  enum insn_code icode = CODE_FOR_atomic_test_and_set;
+  enum machine_mode imode = insn_data[icode].operand[0].mode;
+
+  if (target == NULL_RTX || GET_MODE (target) != imode)
+    target = gen_reg_rtx (imode);
+
+  emit_insn (gen_atomic_test_and_set (target, mem, GEN_INT (model)));
+
+  return target;
+
 #endif
+}
 
 /* This function expands the legacy _sync_lock test_and_set operation which is
    generally an atomic exchange.  Some limited targets only allow the
@@ -7464,11 +7489,8 @@ 
 
   /* If there are no other options, try atomic_test_and_set if the value
      being stored is 1.  */
-  if (!ret && val == const1_rtx && HAVE_atomic_test_and_set)
-    {
-      ret = gen_atomic_test_and_set (target, mem, GEN_INT (MEMMODEL_ACQUIRE));
-      emit_insn (ret);
-    }
+  if (!ret && val == const1_rtx)
+    ret = maybe_emit_atomic_test_and_set (target, mem, MEMMODEL_ACQUIRE);
 
   return ret;
 }
@@ -7488,16 +7510,12 @@ 
   if (target == NULL_RTX)
     target = gen_reg_rtx (mode);
 
-  if (HAVE_atomic_test_and_set)
-    {
-      ret = gen_atomic_test_and_set (target, mem, GEN_INT (MEMMODEL_ACQUIRE));
-      emit_insn (ret);
-      return ret;
-    }
+  ret = maybe_emit_atomic_test_and_set (target, mem, MEMMODEL_ACQUIRE);
 
   /* 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)
+    ret = maybe_emit_atomic_exchange (target, mem, const1_rtx, model);
 
   if (!ret)
     ret = maybe_emit_compare_and_swap_exchange_loop (target, mem, const1_rtx);