RFR (M) 8186903: Remove j-types from Atomic

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Thu Dec 7 04:00:23 UTC 2017



On 12/6/17 9:41 PM, David Holmes wrote:
> Hi Coleen,
>
> On 7/12/2017 7:59 AM, coleen.phillimore at oracle.com wrote:
>> Summary: Change parameter types from jlong, jint, jbyte to int64_t, 
>> int32_t and int8_t respectively
>
> That's fine. But you also made a bunch of changes to rename the "ptr" 
> variants to "long", and in the process changed intptr_t to int64_t. 
> The change ptr->long isn't really accurate. And intptr_t and int64_t 
> are different sizes on 32-bit! So this change seems problematic and 
> out of scope by your own description. That said I thought we had/were 
> getting rid of the "ptr" variants ??

Yes, I did rename atomic_add_ptr_entry to atomic_add_long_entry. This is 
to match the other names which are currently empty, "long" and "byte".   
This was a last vestige of the ptr variants.   It's only used on 64 bit 
windows, so changing it to intptr_t to int64_t is correct.   For some 
reason, there isn't a long variant on 32 bits.  Possibly because we 
don't add jlong values in the vm?
ent
So this renaming makes it consistent with the other names, which then 
can all be changed at once, when nice names are agreed upon.
>
>> Leave renaming functions for later change.
>>
>> Ran JPRT which builds more Oracle platforms, mach5 tier1-5 on 
>> windows/linux x64 and tier1 on solaris-sparcv9.  Also built Zero 
>> product mode (fails building fastdebug for some other reason).
>>
>> Other platforms: could you please test this patch?
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/8186903.01/webrev
>> bug link https://bugs.openjdk.java.net/browse/JDK-8186903
>>
>> This change is for JDK 11.
>
> src/hotspot/cpu/x86/stubGenerator_x86_32.cpp
>
> Return type should be int32_t not int
>
> src/hotspot/os_cpu/windows_x86/os_windows_x86.hpp
>
> -  static intptr_t  (*atomic_xchg_long_func)     (jlong, volatile 
> jlong*);
> -  static intptr_t  atomic_xchg_long_bootstrap   (jlong, volatile 
> jlong*);
>
> Wow - that looks like a bug! Only returns 32-bits on 32-bit!

This one also is only used on 64 bit.  I have to confess I don't follow 
all the paths of the atomics and their various translations down to the 
platform levels.   I'm not sure how windows 32 bit calls atomic_add or 
xchg for 64 bit values (or if it is not implemented).
>
> Everything else seems fine.
>

Thanks for the eagle eye review.
Coleen
> Thanks,
> David
>
>> Thanks,
>> Coleen



More information about the hotspot-runtime-dev mailing list