[PATCH, i386] Fix PR55981, atomic store is split in two smaller stores
Uros Bizjak
ubizjak@gmail.com
Wed Jan 16 17:27:00 GMT 2013
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 <ubizjak@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 <ubizjak@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.
-------------- next part --------------
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 } } */
More information about the Gcc-patches
mailing list