Adding atomic ops for MIPS64.

base/atomicops_internals_mips_gcc.h

Index: base/atomicops_internals_mips_gcc.h
diff --git a/base/atomicops_internals_mips_gcc.h b/base/atomicops_internals_mips_gcc.h
index 29947b37afa61bd48e9ee13f2a47c04ab9d1a5de..20b89a54f0cab8301a8dbff8e75214b03906dd2b 100644
--- a/base/atomicops_internals_mips_gcc.h
+++ b/base/atomicops_internals_mips_gcc.h
@@ -148,6 +148,132 @@ inline Atomic32 Release_Load(volatile const Atomic32* ptr) {
   return *ptr;
 }
 
+#if defined(__LP64__)
+// 64-bit versions of the atomic ops.
+
+inline Atomic64 NoBarrier_CompareAndSwap(volatile Atomic64* ptr,
+                                         Atomic64 old_value,
+                                         Atomic64 new_value) {
+  Atomic64 prev, tmp;
+  __asm__ __volatile__(".set push\n"
+                       ".set noreorder\n"
+                       "1:\n"
+                       "lld %0, %5\n"  // prev = *ptr

[agl, 2014/08/12 18:14:22]

Can you post a disassembly of this function? (objdump -d on the .o file and
trim, for example.)

%5 is asking for the memory location of *ptr. I've checked on x86-64 and GCC
uses |ptr| for this. However, I think it would be completely valid for it to
make a copy of *ptr and use the location of the copy. Why wouldn't you have %5
be "r" (ptr) and then "lld %0, (%5)" to eliminate this?

Also, it appears that the first argument is the destination in this asm. Which
is backwards from the usual AT&T syntax! (And AT&T syntax was supposed to be a
"universal" asm syntax framework.) However, if this compiles then I'm assuming
that it must be correct.

[gordanac, 2014/08/12 20:30:22]

Here it is:
0000000000000000 <NoBarrier_CompareAndSwap(int volatile*, int, int)>:
   0:   67bdffd0        daddiu  sp,sp,-48
   4:   ffbe0028        sd      s8,40(sp)
   8:   03a0f02d        move    s8,sp
   c:   ffc40010        sd      a0,16(s8)
  10:   00a0182d        move    v1,a1
  14:   00c0102d        move    v0,a2
  18:   00031800        sll     v1,v1,0x0
  1c:   afc30018        sw      v1,24(s8)
  20:   00021000        sll     v0,v0,0x0
  24:   afc2001c        sw      v0,28(s8)
  28:   dfc60010        ld      a2,16(s8)
  2c:   8fc20018        lw      v0,24(s8)
  30:   8fc3001c        lw      v1,28(s8)
  34:   dfc40010        ld      a0,16(s8)
  38:   7c850037        lld     a1,0(a0)
  3c:   14a20000        bne     a1,v0,40 <NoBarrier_CompareAndSwap(int
volatile*, int, int)+0x40>
  40:   0060382d        move    a3,v1
  44:   7cc70027        scd     a3,0(a2)
  48:   10e00000        beqz    a3,4c <NoBarrier_CompareAndSwap(int volatile*,
int, int)+0x4c>
  4c:   00000000        nop
  50:   00e0202d        move    a0,a3
  54:   afc50000        sw      a1,0(s8)
  58:   afc40004        sw      a0,4(s8)
  5c:   8fc20000        lw      v0,0(s8)
  60:   03c0e82d        move    sp,s8
  64:   dfbe0028        ld      s8,40(sp)
  68:   67bd0030        daddiu  sp,sp,48
  6c:   03e00009        jr      ra
  70:   00000000        nop

[agl, 2014/08/12 21:05:49]

That seems sane (although not optimised). But don't you agree that asking for
"m" (*ptr) in order to get ptr is weird and probably error prone? It already has
a memory clobber.

[petarj, 2014/08/12 23:40:40]

This function gets inlined, so the code is more optimal at places where
it is used.

As of your question of "lld %0, %5", AFAICS glibc does it [1] the same
way, so I'd vote we keep it in that fashion.

Regards,
Petar

[1] glibc, MIPS atomics
https://sourceware.org/git/?p=glibc.git;a=blob_plain;f=sysdeps/mips/bits/atom...

[agl, 2014/08/13 17:52:37]

Fair enough. glibc doing it should mean that any compiler change that breaks it
gets noticed before we hit it.

+                       "bne %0, %3, 2f\n"  // if (prev != old_value) goto 2
+                       "move %2, %4\n"  // tmp = new_value
+                       "scd %2, %1\n"  // *ptr = tmp (with atomic check)
+                       "beqz %2, 1b\n"  // start again on atomic error
+                       "nop\n"  // delay slot nop
+                       "2:\n"
+                       ".set pop\n"
+                       : "=&r" (prev), "=m" (*ptr), "=&r" (tmp)
+                       : "Ir" (old_value), "r" (new_value), "m" (*ptr)
+                       : "memory");
+  return prev;
+}
+
+// Atomically store new_value into *ptr, returning the previous value held in
+// *ptr.  This routine implies no memory barriers.
+inline Atomic64 NoBarrier_AtomicExchange(volatile Atomic64* ptr,
+                                         Atomic64 new_value) {
+  Atomic64 temp, old;
+  __asm__ __volatile__(".set push\n"
+                       ".set noreorder\n"
+                       "1:\n"
+                       "lld %1, %2\n"  // old = *ptr

[agl, 2014/08/13 17:52:37]

The %2 here is an output argument being used as an input to lld, right? That
seems wrong (and the glibc sources for the same use an input argument here.)

This has been duplicated from the version above so please fix there too if you
change this.

[gordanac, 2014/08/14 12:09:33]

Done.

+                       "move %0, %3\n"  // temp = new_value
+                       "scd %0, %2\n"  // *ptr = temp (with atomic check)
+                       "beqz %0, 1b\n"  // start again on atomic error
+                       "nop\n"  // delay slot nop
+                       ".set pop\n"
+                       : "=&r" (temp), "=&r" (old), "=m" (*ptr)
+                       : "r" (new_value), "m" (*ptr)
+                       : "memory");
+
+  return old;
+}
+
+// Atomically increment *ptr by "increment".  Returns the new value of
+// *ptr with the increment applied.  This routine implies no memory barriers.
+inline Atomic64 NoBarrier_AtomicIncrement(volatile Atomic64* ptr,
+                                          Atomic64 increment) {
+  Atomic64 temp, temp2;
+
+  __asm__ __volatile__(".set push\n"
+                       ".set noreorder\n"
+                       "1:\n"
+                       "lld %0, %2\n"  // temp = *ptr

[agl, 2014/08/13 17:52:37]

ditto.

[gordanac, 2014/08/14 12:09:33]

Done.

+                       "daddu %1, %0, %3\n"  // temp2 = temp + increment
+                       "scd %1, %2\n"  // *ptr = temp2 (with atomic check)
+                       "beqz %1, 1b\n"  // start again on atomic error
+                       "daddu %1, %0, %3\n"  // temp2 = temp + increment
+                       ".set pop\n"
+                       : "=&r" (temp), "=&r" (temp2), "=m" (*ptr)
+                       : "Ir" (increment), "m" (*ptr)
+                       : "memory");
+  // temp2 now holds the final value.
+  return temp2;
+}
+
+inline Atomic64 Barrier_AtomicIncrement(volatile Atomic64* ptr,
+                                        Atomic64 increment) {
+  MemoryBarrier();
+  Atomic64 res = NoBarrier_AtomicIncrement(ptr, increment);
+  MemoryBarrier();
+  return res;
+}
+
+// "Acquire" operations
+// ensure that no later memory access can be reordered ahead of the operation.
+// "Release" operations ensure that no previous memory access can be reordered
+// after the operation.  "Barrier" operations have both "Acquire" and "Release"
+// semantics.   A MemoryBarrier() has "Barrier" semantics, but does no memory
+// access.
+inline Atomic64 Acquire_CompareAndSwap(volatile Atomic64* ptr,
+                                       Atomic64 old_value,
+                                       Atomic64 new_value) {
+  Atomic64 res = NoBarrier_CompareAndSwap(ptr, old_value, new_value);
+  MemoryBarrier();
+  return res;
+}
+
+inline Atomic64 Release_CompareAndSwap(volatile Atomic64* ptr,
+                                       Atomic64 old_value,
+                                       Atomic64 new_value) {
+  MemoryBarrier();
+  return NoBarrier_CompareAndSwap(ptr, old_value, new_value);
+}
+
+inline void NoBarrier_Store(volatile Atomic64* ptr, Atomic64 value) {
+  *ptr = value;
+}
+
+inline void Acquire_Store(volatile Atomic64* ptr, Atomic64 value) {
+  *ptr = value;
+  MemoryBarrier();
+}
+
+inline void Release_Store(volatile Atomic64* ptr, Atomic64 value) {
+  MemoryBarrier();
+  *ptr = value;
+}
+
+inline Atomic64 NoBarrier_Load(volatile const Atomic64* ptr) {
+  return *ptr;
+}
+
+inline Atomic64 Acquire_Load(volatile const Atomic64* ptr) {
+  Atomic64 value = *ptr;
+  MemoryBarrier();
+  return value;
+}
+
+inline Atomic64 Release_Load(volatile const Atomic64* ptr) {
+  MemoryBarrier();
+  return *ptr;
+}
+#endif
+
 } // namespace base::subtle
 } // namespace base