diff mbox

[RFA] : Properly mask memory model argument with MEMMODEL_MASK

Message ID CAFULd4Ze2O0htQD2+8p+7qT7+KcdSAp5uusnR7UroSvK4nCiQg@mail.gmail.com
State New
Headers show

Commit Message

Uros Bizjak Jan. 15, 2013, 8:09 a.m. UTC 
Hello!

Attached fairly mechanical patch adds missing memory model argument
masks when comparing comparing with various MEMMODEL_* types. Please
note that we can modify the memory model with target flags, IX86_HLE_*
in case of x86.

Also, please note that this patch doesn't touch tsan.c.

2013-01-15  Uros Bizjak  <ubizjak@gmail.com>

        * emit-rtl.c (need_atomic_barrier_p): Mask memory model argument
        with MEMMODEL_MASK before comparing with MEMMODEL_* memory types.
        * optabs.c (maybe_emit_sync_lock_test_and_set): Ditto.
        (expand_mem_thread_fence): Ditto.
        (expand_mem_signal_fence): Ditto.
        (expand_atomic_load): Ditto.
        (expand_atomic_store): Ditto.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

OK for mainline?

Uros.

Comments

Andi Kleen Jan. 15, 2013, 8:37 p.m. UTC | #1 
Uros Bizjak <ubizjak@gmail.com> writes:
>
> 2013-01-15  Uros Bizjak  <ubizjak@gmail.com>
>
>         * emit-rtl.c (need_atomic_barrier_p): Mask memory model argument
>         with MEMMODEL_MASK before comparing with MEMMODEL_* memory types.
>         * optabs.c (maybe_emit_sync_lock_test_and_set): Ditto.
>         (expand_mem_thread_fence): Ditto.
>         (expand_mem_signal_fence): Ditto.
>         (expand_atomic_load): Ditto.
>         (expand_atomic_store): Ditto.
>
> Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
>
> OK for mainline?

Looks good to me (but I cannot approve)

-Andi
Richard Henderson Jan. 15, 2013, 9:05 p.m. UTC | #2 
On 01/15/2013 12:09 AM, Uros Bizjak wrote:
> 2013-01-15  Uros Bizjak<ubizjak@gmail.com>
>
>          * emit-rtl.c (need_atomic_barrier_p): Mask memory model argument
>          with MEMMODEL_MASK before comparing with MEMMODEL_* memory types.
>          * optabs.c (maybe_emit_sync_lock_test_and_set): Ditto.
>          (expand_mem_thread_fence): Ditto.
>          (expand_mem_signal_fence): Ditto.
>          (expand_atomic_load): Ditto.
>          (expand_atomic_store): Ditto.

Ok.


r~
diff mbox

Patch

Index: emit-rtl.c
===================================================================
--- emit-rtl.c	(revision 195152)
+++ emit-rtl.c	(working copy)
@@ -6014,7 +6014,7 @@  insn_file (const_rtx insn)
 bool
 need_atomic_barrier_p (enum memmodel model, bool pre)
 {
-  switch (model)
+  switch (model & MEMMODEL_MASK)
     {
     case MEMMODEL_RELAXED:
     case MEMMODEL_CONSUME:
Index: optabs.c
===================================================================
--- optabs.c	(revision 195152)
+++ optabs.c	(working copy)
@@ -7008,9 +7008,9 @@  maybe_emit_sync_lock_test_and_set (rtx target, rtx
      exists, and the memory model is stronger than acquire, add a release 
      barrier before the instruction.  */
 
-  if (model == MEMMODEL_SEQ_CST
-      || model == MEMMODEL_RELEASE
-      || model == MEMMODEL_ACQ_REL)
+  if ((model & MEMMODEL_MASK) == MEMMODEL_SEQ_CST
+      || (model & MEMMODEL_MASK) == MEMMODEL_RELEASE
+      || (model & MEMMODEL_MASK) == MEMMODEL_ACQ_REL)
     expand_mem_thread_fence (model);
 
   if (icode != CODE_FOR_nothing)
@@ -7388,7 +7388,7 @@  expand_mem_thread_fence (enum memmodel model)
 {
   if (HAVE_mem_thread_fence)
     emit_insn (gen_mem_thread_fence (GEN_INT (model)));
-  else if (model != MEMMODEL_RELAXED)
+  else if ((model & MEMMODEL_MASK) != MEMMODEL_RELAXED)
     {
       if (HAVE_memory_barrier)
 	emit_insn (gen_memory_barrier ());
@@ -7412,7 +7412,7 @@  expand_mem_signal_fence (enum memmodel model)
 {
   if (HAVE_mem_signal_fence)
     emit_insn (gen_mem_signal_fence (GEN_INT (model)));
-  else if (model != MEMMODEL_RELAXED)
+  else if ((model & MEMMODEL_MASK) != MEMMODEL_RELAXED)
     {
       /* By default targets are coherent between a thread and the signal
 	 handler running on the same thread.  Thus this really becomes a
@@ -7467,7 +7467,7 @@  expand_atomic_load (rtx target, rtx mem, enum memm
     target = gen_reg_rtx (mode);
 
   /* For SEQ_CST, emit a barrier before the load.  */
-  if (model == MEMMODEL_SEQ_CST)
+  if ((model & MEMMODEL_MASK) == MEMMODEL_SEQ_CST)
     expand_mem_thread_fence (model);
 
   emit_move_insn (target, mem);
@@ -7513,7 +7513,7 @@  expand_atomic_store (rtx mem, rtx val, enum memmod
 	  if (maybe_expand_insn (icode, 2, ops))
 	    {
 	      /* lock_release is only a release barrier.  */
-	      if (model == MEMMODEL_SEQ_CST)
+	      if ((model & MEMMODEL_MASK) == MEMMODEL_SEQ_CST)
 		expand_mem_thread_fence (model);
 	      return const0_rtx;
 	    }
@@ -7540,7 +7540,7 @@  expand_atomic_store (rtx mem, rtx val, enum memmod
   emit_move_insn (mem, val);
 
   /* For SEQ_CST, also emit a barrier after the store.  */
-  if (model == MEMMODEL_SEQ_CST)
+  if ((model & MEMMODEL_MASK) == MEMMODEL_SEQ_CST)
     expand_mem_thread_fence (model);
 
   return const0_rtx;