[Openmp-commits] [PATCH] D19878: Use C++11 atomics for ticket locks implementation

[John Mellor-Crummey via Openmp-commits]

jmellorcrummey added a comment.

I have a few more comments about the memory orderings. Not having too much order is important for performance. Fences that are inserted unless they are explicitly omitted hurt performance.

After you see my list below, you'll be wondering whether you should have ventured down this path :-)

Line 741 the fetch and add can be relaxed order. the acquire order is important on the load that tests vs. myticket
Line 744 this load isn't for synchronization, it can be relaxed order
Line 749 this atomic load should have acquire order, but does not need to be sequentially consistent
Line 767 this load isn't for synchronization, it can be relaxed order
Line 782 this store isn't for synchronization, the store can be relaxed order
Line 789 this load isn't for synchronization, it can be relaxed order
Line 791 this load can be relaxed because the CAS on line 793 enfoceces the acquire order
Line 806 this load isn't for synchronization, it can be relaxed order
Line 782 this store isn't for synchronization, the store can be relaxed order
Line 827 neither of these loads are for synchronization, so they can be relaxed
Line 840 this load isn't for synchronization, it can be relaxed order
Lines 865-869 all of these stores can be relaxed
Line 870 if this store is release order,  all of the prior stores in this routine will complete before the initialized flag is set
Line 882 could be release order to ensure all prior operations on the lock are complete.
Line 883-888 these stores can be relaxed. I could argue that once you clear initialized, you could not bother writing the rest of the values.
Line 986 this load can be relaxed because it isn't for synchronization
Line 922 can be relaxed. all ordering will be enforced by synchronizing accesses for the outer lock
Line 927-928 can be relaxed. these aren't synchronizing.
Line 938 can be relaxed. it isn't synchronizing.
Line 958 can be relaxed. all ordering will be enforced by synchronizing accesses for the outer lock
Lines 964-965 can be relaxed.
Line 977 can be relaxed. it isn't synchronizing.
Lines 994-995 aren't synchronizing and can be relaxed.
Line 1007 isn't synchronizing and can be relaxed.
Line 1029 can be relaxed (or even omitted)
Line 1042 can be relaxed (or even omitted)
Line 1050 can be relaxed
Line 1073 can be relaxed


Repository:
  rL LLVM

http://reviews.llvm.org/D19878