RFR: 8186166: Generalize Atomic::cmpxchg with templates

David Holmes david.holmes at oracle.com
Wed Aug 16 21:13:37 UTC 2017


On 17/08/2017 6:38 AM, Kim Barrett wrote:
>> On Aug 16, 2017, at 2:03 AM, Kim Barrett <kim.barrett at oracle.com> wrote:
>>
>>> On Aug 15, 2017, at 2:57 AM, David Holmes <david.holmes at oracle.com> wrote:src/os_cpu/aix_ppc/vm/atomic_aix_ppc.hpp
>>>
>>> The actual body has:
>>>
>>> 435   long old_value;
>>>
>>> rather than "T old_value". I presume the AIX compiler can't handle using T with inline asm?
>>
>> There wasn't any good reason not to do that, that I know of, so I've
>> made that change.  Similarly for linux_ppc.  I made a similar change
>> to linux_s390 for the 8-byte case, but not 4-byte, where the existing
>> code is declaring old to be unsigned long rather than the expected
>> unsigned int.  I don't know if that's a bug or something about the
>> s390.  Looking at the s390 code, I think there might be missing
>> compile barriers that allow (non-volatile) memory accesses to be
>> reordered across the operation.  I'll bring those issues up directly
>> with the SAP folks.
> 
> Martin Doerr has confirmed the s390 4-byte cmpxchg’s use of unsigned long
> rather than unsigned int is a (harmless) mistake, so I’ve made the same changes
> there as elsewhere now.  (No new webrev for this change, the diff is below.)  SAP
> will address the compile barrier issue.

Looks good. Glad we got that tidied up. :)

Thanks,
David

> ==========
> diff -r ece36cba61c1 -r f203533cabc6 src/os_cpu/linux_s390/vm/atomic_linux_s390.hpp
> --- a/src/os_cpu/linux_s390/vm/atomic_linux_s390.hpp	Tue Aug 15 19:50:03 2017 -0400
> +++ b/src/os_cpu/linux_s390/vm/atomic_linux_s390.hpp	Wed Aug 16 13:17:52 2017 -0400
> @@ -489,7 +489,7 @@
>                                                   T cmp_val,
>                                                   cmpxchg_memory_order unused) const {
>     STATIC_ASSERT(4 == sizeof(T));
> -  unsigned long old;
> +  T old;
>   
>     __asm__ __volatile__ (
>       "   CS       %[old],%[upd],%[mem]    \n\t" // Try to xchg upd with mem.
> @@ -503,7 +503,7 @@
>       : "cc"
>     );
>   
> -  return IntegerTypes::cast<T>((uint32_t)old);
> +  return old;
>   }
>   
>   template<>
> ==========
> 


More information about the hotspot-dev mailing list