RFR(M): 8155949: Support relaxed semantics in cmpxchg

David Holmes david.holmes at oracle.com
Fri May 20 23:09:31 UTC 2016


Hi Martin,

Are you in a position to make the change now suggested by both Kim and 
Andrew? Can you also include the Aarch64 code that Andrew provided:

http://cr.openjdk.java.net/~aph/8154736

I'd like to get this finalized so it is ready to push as soon as the 
process allows it to.

Thanks,
David

On 20/05/2016 8:03 AM, Kim Barrett wrote:
>> On May 18, 2016, at 6:12 AM, Doerr, Martin <martin.doerr at sap.com> wrote:
>>
>> Hi Kim,
>>
>> thank you very much for the detailed review.
>>
>> I agree with your comments and I have made all your requested changes here:
>> http://cr.openjdk.java.net/~goetz/wr16/8155949-relaxed_cas/webrev.03/
>>
>> It's correct that the change changes the semantics of the conservative cmpxchg. In case of failure, we also execute the sync instruction, now.
>> Advantage is that the new implementation is maximum conservative by default. I think this makes sense as long as the semantics of the hotspot C++ cmpxchg are not clearly specified.
>>
>> For performance optimization, we should better use (or introduce additional) enum values.
>
> ------------------------------------------------------------------------------
> There doesn't seem to have been any change for this earlier comment.
>
> src/share/vm/runtime/atomic.cpp
> 59 unsigned Atomic::cmpxchg(unsigned int exchange_value,
>  60                            volatile unsigned int* dest, unsigned int compare_value,
>  61                            cmpxchg_memory_order order) {
>
> I'm surprised this was ever out-of-line. But with this change it's
> quite bad to be out-of-line, as that's going to kill the constant
> propogation of the order value.
>
> ------------------------------------------------------------------------------
>
> Other than that, looks good.
>
>
>
>


More information about the hotspot-runtime-dev mailing list