RFR (M) 8186903: Remove j-types from Atomic
coleen.phillimore at oracle.com
coleen.phillimore at oracle.com
Thu Dec 7 04:27:35 UTC 2017
resending.
On 12/6/17 11:00 PM, coleen.phillimore at oracle.com wrote:
>
>
> 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-dev
mailing list