[MIPS] Fix memory barriers for atomic operations.

[MIPS] Fix memory barriers for atomic operations.

Add barriers using MIPS 'sync' instructions as needed for SMP
systems.

BUG=246947
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=204697

[paul.l..., 2013-06-06 04:32:26]

PTAL at this initial CL. The comments and definitions in atomicops.h for
Acquire_CompareAndSwap() and Release_CompareAndSwap() and
Barrier_AtomicIncrement() imply that the 'sync' is needed. But that is an
expensive operation, so let's make certain we really need to do this.

[Dmitry Vyukov, 2013-06-06 08:14:44]

https://codereview.chromium.org/16001009/diff/1/base/atomicops_internals_mips...
File base/atomicops_internals_mips_gcc.h (right):

https://codereview.chromium.org/16001009/diff/1/base/atomicops_internals_mips...
base/atomicops_internals_mips_gcc.h:108: MemoryBarrier();
It must be the other way around:
For acquire: memory operation followed by barrier.
For release: memory operation preceded by barrier.

[dusmil, 2013-06-06 13:35:15]

That's right, it should be in different order.

[paul.l..., 2013-06-06 17:07:03]

Fixed barrier ordering.

https://codereview.chromium.org/16001009/diff/1/base/atomicops_internals_mips...
File base/atomicops_internals_mips_gcc.h (right):

https://codereview.chromium.org/16001009/diff/1/base/atomicops_internals_mips...
base/atomicops_internals_mips_gcc.h:108: MemoryBarrier();
Done. Thanks for the review!

[Dmitry Vyukov, 2013-06-06 17:45:10]

LGTM

https://codereview.chromium.org/16001009/diff/6001/base/atomicops_internals_m...
File base/atomicops_internals_mips_gcc.h (right):

https://codereview.chromium.org/16001009/diff/6001/base/atomicops_internals_m...
base/atomicops_internals_mips_gcc.h:117: Atomic32 res =
NoBarrier_CompareAndSwap(ptr, old_value, new_value);
now this can be just
return NoBarrier_CompareAndSwap

[paul.l..., 2013-06-06 21:02:11]

Thanks Dmitry for the reviews. Adding David and Mark as reviewers. Should I add
any other owners besides Mark?

https://codereview.chromium.org/16001009/diff/6001/base/atomicops_internals_m...
File base/atomicops_internals_mips_gcc.h (right):

https://codereview.chromium.org/16001009/diff/6001/base/atomicops_internals_m...
base/atomicops_internals_mips_gcc.h:117: Atomic32 res =
NoBarrier_CompareAndSwap(ptr, old_value, new_value);
Of course... Done.

[Mark Mentovai, 2013-06-06 21:49:03]

LGTM

[Mark Mentovai, 2013-06-06 21:50:19]

https://codereview.chromium.org/16001009/diff/11001/base/atomicops_internals_...
File base/atomicops_internals_mips_gcc.h (right):

https://codereview.chromium.org/16001009/diff/11001/base/atomicops_internals_...
base/atomicops_internals_mips_gcc.h:12: #define ATOMICOPS_COMPILER_BARRIER()
__asm__ __volatile__("" : : : "memory")
Please remove this macro now, along with the #undef at the bottom of the file.

[paul.l..., 2013-06-06 21:57:51]

Done. Thanks for the review.

https://codereview.chromium.org/16001009/diff/11001/base/atomicops_internals_...
File base/atomicops_internals_mips_gcc.h (right):

https://codereview.chromium.org/16001009/diff/11001/base/atomicops_internals_...
base/atomicops_internals_mips_gcc.h:12: #define ATOMICOPS_COMPILER_BARRIER()
__asm__ __volatile__("" : : : "memory")
On 2013/06/06 21:50:19, Mark Mentovai wrote:
> Please remove this macro now, along with the #undef at the bottom of the file.

Done.

[Mark Mentovai, 2013-06-06 22:00:31]

LGTM. You don’t need any more reviews.

[commit-bot: I haz the power, 2013-06-06 22:05:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/paul.lind@imgtec.com/16001009/17001

[commit-bot: I haz the power, 2013-06-07 02:31:17]

Change committed as 204697