Hello!

Using plain movdi pattern is not guaranteed to be atomic for all
operands. For x86_64, when storing DImode immediate outside SImode
range, the compiler splits the move into two separate SImode moves,
violating atomic assumptions.

Attached patch generates atomic store for all supported input arguments.

A related questions about volatile stores:
- Does language standard guarantee atomic store in this case
[wikipedia says "No." [1]]?
- Can a store to a volatile DImode location be implemented as two
consecutive SImode stores to adjacent location (this breaks stores to
MMIO 64bit registers)?

2012-01-16  Uros Bizjak  <ubiz...@gmail.com>

        PR target/55981
        * config/i386/sync.md (atomic_store<mode>): Always generate SWImode
        store through atomic_store<mode>_1.
        (atomic_store<mode>_1): Macroize insn using SWI mode iterator.

testsuite/ChangeLog:

2012-01-16  Uros Bizjak  <ubiz...@gmail.com>

        PR target/55981
        * gcc.target/pr55981.c: New test.

Tested on x86_64-pc-linux-gnu.

I will wait a couple of days for possible comments.

[1] http://en.wikipedia.org/wiki/Volatile_variable#In_C_and_C.2B.2B

Uros.
Index: config/i386/sync.md
===================================================================
--- config/i386/sync.md (revision 195240)
+++ config/i386/sync.md (working copy)
@@ -225,11 +225,8 @@
        }
 
       /* Otherwise use a store.  */
-      if (INTVAL (operands[2]) & IX86_HLE_RELEASE)
-       emit_insn (gen_atomic_store<mode>_1 (operands[0], operands[1],
-                                            operands[2]));
-      else
-       emit_move_insn (operands[0], operands[1]);
+      emit_insn (gen_atomic_store<mode>_1 (operands[0], operands[1],
+                                          operands[2]));
     }
   /* ... followed by an MFENCE, if required.  */
   if (model == MEMMODEL_SEQ_CST)
@@ -238,10 +235,10 @@
 })
 
 (define_insn "atomic_store<mode>_1"
-  [(set (match_operand:ATOMIC 0 "memory_operand" "=m")
-       (unspec:ATOMIC [(match_operand:ATOMIC 1 "<nonmemory_operand>" "<r><i>")
-                       (match_operand:SI 2 "const_int_operand")]
-                      UNSPEC_MOVA))]
+  [(set (match_operand:SWI 0 "memory_operand" "=m")
+       (unspec:SWI [(match_operand:SWI 1 "<nonmemory_operand>" "<r><i>")
+                    (match_operand:SWI 2 "const_int_operand")]
+                   UNSPEC_MOVA))]
   ""
   "%K2mov{<imodesuffix>}\t{%1, %0|%0, %1}")
 
Index: testsuite/gcc.target/i386/pr55981.c
===================================================================
--- testsuite/gcc.target/i386/pr55981.c (revision 0)
+++ testsuite/gcc.target/i386/pr55981.c (working copy)
@@ -0,0 +1,54 @@
+/* { dg-do compile  { target { ! { ia32 } } } } */
+/* { dg-options "-O2" } */
+
+volatile int a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p;
+
+volatile long long y;
+
+void
+test ()
+{
+  int a_ = a;
+  int b_ = b;
+  int c_ = c;
+  int d_ = d;
+  int e_ = e;
+  int f_ = f;
+  int g_ = g;
+  int h_ = h;
+  int i_ = i;
+  int j_ = j;
+  int k_ = k;
+  int l_ = l;
+  int m_ = m;
+  int n_ = n;
+  int o_ = o;
+  int p_ = p;
+
+  int z;
+
+  for (z = 0; z < 1000; z++)
+    {
+      __atomic_store_n (&y, 0x100000002ll, __ATOMIC_SEQ_CST);
+      __atomic_store_n (&y, 0x300000004ll, __ATOMIC_SEQ_CST);
+    }
+
+  a = a_;
+  b = b_;
+  c = c_;
+  d = d_;
+  e = e_;
+  f = f_;
+  g = g_;
+  h = h_;
+  i = i_;
+  j = j_;
+  k = k_;
+  l = l_;
+  m = m_;
+  n = n_;
+  o = o_;
+  p = p_;
+}
+
+/* { dg-final { scan-assembler-times "movabs" 2 } } */

Reply via email to