[aarch64-port-dev ] RFR: 8243339: AArch64: Obsolete UseBarriersForVolatile option

[Derek White]

Xiaohong, thanks for including the ThunderX1 cleanup!
 - Derek

-----Original Message-----
From: Xiaohong Gong <Xiaohong.Gong at arm.com> 
Sent: Wednesday, May 27, 2020 3:23 AM
To: Ningsheng Jian <Ningsheng.Jian at arm.com>; Andrew Haley <aph at redhat.com>; Andrew Dinn <adinn at redhat.com>; Derek White <derekw at marvell.com>; aarch64-port-dev at openjdk.java.net; hotspot-compiler-dev at openjdk.java.net
Cc: nd <nd at arm.com>
Subject: [EXT] RE: [aarch64-port-dev ] RFR: 8243339: AArch64: Obsolete UseBarriersForVolatile option

----------------------------------------------------------------------
Hi Ningsheng,

Thanks for the pushing!

Best Regards,
Xiaohong Gong

-----Original Message-----
From: Ningsheng Jian <ningsheng.jian at arm.com>
Sent: Wednesday, May 27, 2020 3:21 PM
To: Xiaohong Gong <Xiaohong.Gong at arm.com>; Andrew Haley <aph at redhat.com>; Andrew Dinn <adinn at redhat.com>; Derek White <derekw at marvell.com>; aarch64-port-dev at openjdk.java.net; hotspot-compiler-dev at openjdk.java.net
Cc: nd <nd at arm.com>
Subject: Re: [aarch64-port-dev ] RFR: 8243339: AArch64: Obsolete UseBarriersForVolatile option

I see CSR review and submit tests are clear, so I pushed.

Thanks,
Ningsheng

On 5/22/20 10:36 AM, Xiaohong Gong wrote:
> Hi Andrew,
> 
>> On 5/21/20 11:24 AM, Xiaohong Gong wrote:
>   > > I'v created a new patch to add the condition when inserting
>   > "DMBs"
>   > > before volatile load for C1/Interpreter.
>   > > The updated webrev:
>   > > https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Exgong_rfr_8243339_webrev.01_&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=gW0hANMfJfyELYt_X2mceubwzCNjT0vmaU97kngYUJk&m=v4awzqAGspetRbA0GcI6qzWI4gEqvTZJanovmjlaOfc&s=usotLgy1ZYkvIvvmvMzZBgyVA8tbxdiENiMKxe-xqV8&e= 
>   > >
>   > > It adds a new function "is_c1_or_interpreter_only()" , which can
>   > > decide whether C2/JVMCI is used. Besides, since AOT also uses
>   > Graal
>   > > compiler as the codegen, it always return false if AOT mode is
>   > enabled.
>   >
>   > Looks good to me, thanks.
>   >
>   > As far as I remember, Graal does optimize volatile accesses to use
>   > ldar/stlr, or at least it will do so in the future, so if we're
>   > using AOT or JVMCI the safe thing to do is add the DMBs.
> 
> Yes, exactly! It has a patch in Graal github to do this optimization (https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_oracle_graal_pull_1772&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=gW0hANMfJfyELYt_X2mceubwzCNjT0vmaU97kngYUJk&m=v4awzqAGspetRbA0GcI6qzWI4gEqvTZJanovmjlaOfc&s=BD3vlImO8sPL4QC9AGdC3Nrs-0FaO3hzd5uhecHCcjQ&e= ).
> 
> Thanks,
> Xiaohong
> 
> -----Original Message-----
> From: Andrew Haley <aph at redhat.com>
> Sent: Thursday, May 21, 2020 10:06 PM
> To: Xiaohong Gong <Xiaohong.Gong at arm.com>; Andrew Dinn 
> <adinn at redhat.com>; Derek White <derekw at marvell.com>; 
> aarch64-port-dev at openjdk.java.net;
> hotspot-compiler-dev at openjdk.java.net
> Cc: nd <nd at arm.com>
> Subject: Re: [aarch64-port-dev ] RFR: 8243339: AArch64: Obsolete 
> UseBarriersForVolatile option
> 
> On 5/21/20 11:24 AM, Xiaohong Gong wrote:
>> I'v created a new patch to add the condition when inserting "DMBs"
>> before volatile load for C1/Interpreter.
>> The updated webrev:
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.n
>> et_-7Exgong_rfr_8243339_webrev.01_&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&
>> r=gW0hANMfJfyELYt_X2mceubwzCNjT0vmaU97kngYUJk&m=v4awzqAGspetRbA0GcI6q
>> zWI4gEqvTZJanovmjlaOfc&s=usotLgy1ZYkvIvvmvMzZBgyVA8tbxdiENiMKxe-xqV8&
>> e=
>>
>> It adds a new function "is_c1_or_interpreter_only()" , which can 
>> decide whether C2/JVMCI is used. Besides, since AOT also uses Graal 
>> compiler as the codegen, it always return false if AOT mode is enabled.
> 
> Looks good to me, thanks.
> 
> As far as I remember, Graal does optimize volatile accesses to use ldar/stlr, or at least it will do so in the future, so if we're using AOT or JVMCI the safe thing to do is add the DMBs.
> 
> --
> Andrew Haley  (he/him)
> Java Platform Lead Engineer
> Red Hat UK Ltd. 
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.redhat.com&d
> =DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=gW0hANMfJfyELYt_X2mceubwzCNjT0vmaU9
> 7kngYUJk&m=v4awzqAGspetRbA0GcI6qzWI4gEqvTZJanovmjlaOfc&s=FzoZCaoTBONlu
> dy6T1fhIPmq8cP9SmBJZGaj8xZrN-g&e= > 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__keybase.io_andrew
> haley&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=gW0hANMfJfyELYt_X2mceubwzCNj
> T0vmaU97kngYUJk&m=v4awzqAGspetRbA0GcI6qzWI4gEqvTZJanovmjlaOfc&s=gD2Ro-
> lXyKC-MfVBk8CWQJ3SXcWYtJ3NGHDOdE9vINc&e=
> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
>