RFR: 8243339: AArch64: Obsolete UseBarriersForVolatile option

[Andrew Dinn]

Hi Xiaohong,

Just for references a direct link to the webrev, issue and CSR are:

  https://cr.openjdk.java.net/~xgong/rfr/8243339/webrev.00/
  https://bugs.openjdk.java.net/browse/JDK-8243339
  https://bugs.openjdk.java.net/browse/JDK-8243456

The webrev looks fine to me. Nice work, thank you!

regards,


Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill


On 13/05/2020 21:20, Derek White wrote:
> Hi Xiaohong,
> 
> This looks good to me (not a (R)eviewer). Thanks for including the patch for ThunderX !
> 
> This is a nice cleanup of the code and especially the volatile tests.
> 
> I did NOT check that the removal of the Compiler Interface declarations does or doesn't impact compilation of the Graal compiler. Did you check that side?
> 
> Thanks,
>  - Derek
> 
> -----Original Message-----
> From: hotspot-compiler-dev <hotspot-compiler-dev-bounces at openjdk.java.net> On Behalf Of Xiaohong Gong
> Sent: Thursday, May 7, 2020 11:39 PM
> To: aarch64-port-dev at openjdk.java.net; hotspot-compiler-dev at openjdk.java.net
> Cc: nd <nd at arm.com>
> Subject: [EXT] RFR: 8243339: AArch64: Obsolete UseBarriersForVolatile option
> 
> External Email
> 
> ----------------------------------------------------------------------
> Hi,
> 
> Please  help to review this patch which obsoletes the product flag "-XX:UseBarrierssForVolatile" and its related code:
> Webrev:
> https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Exgong_rfr_8243339_webrev.00_&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=gW0hANMfJfyELYt_X2mceubwzCNjT0vmaU97kngYUJk&m=OT2KWgejq1kc_YJt_NWWZBKwYqCqlRvzSlfRO04igpk&s=oI-OcgRUa25GiBZaU5V4OmuS8aewSxBaMLbKw3A7lnA&e=
> JBS:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8243339&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=gW0hANMfJfyELYt_X2mceubwzCNjT0vmaU97kngYUJk&m=OT2KWgejq1kc_YJt_NWWZBKwYqCqlRvzSlfRO04igpk&s=T-oDfXrBvQorUBzFZq7Omb17P5yqQjg_q3dBo4EExCA&e=
> https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8243456&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=gW0hANMfJfyELYt_X2mceubwzCNjT0vmaU97kngYUJk&m=OT2KWgejq1kc_YJt_NWWZBKwYqCqlRvzSlfRO04igpk&s=MDMAqUQRO_kmBtmTodGJ2wNuaVy-u_Y_jykpMmyMQwI&e=  (CSR)
> 
> As described in the CSR, using "-XX:+UseBarriersForVolatile" might have memory consistent issue like that mentioned in [1]. It needs more effort to fix the issue and maintain the memory consistency in future. Since "ldar/stlr" has worked well for a long time, and so does "ldaxr/stlxr" for unsafe atomics, we'd better simplify things by removing this option and the alternative implementation for the volatile access.
> 
> Since its only one signifcant usage on a kind of CPU would also like to be removed (See [2]), it can work well without this option. So we directly obsolete this option and remove the code, rather than deprecate it firstly.
> 
> Besides obsoleting this option, this patch also removes an AArch64 CPU feature "CPU_DMB_ATOMICS" together. It is a  workaround while not an AArch64 official feature, which is not required anymore (See [2]).
> 
> [1] https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8241137&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=gW0hANMfJfyELYt_X2mceubwzCNjT0vmaU97kngYUJk&m=OT2KWgejq1kc_YJt_NWWZBKwYqCqlRvzSlfRO04igpk&s=XQwo26nMgDENOKN5U4pW2EunOEt2UtLycucN1BCScaU&e=
> [2] https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8242469&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=gW0hANMfJfyELYt_X2mceubwzCNjT0vmaU97kngYUJk&m=OT2KWgejq1kc_YJt_NWWZBKwYqCqlRvzSlfRO04igpk&s=ChT4b4Jj_TkXozuRs6HIqUVPn1iap0DzKvB-2dKYf0g&e= 
> 
> Testing:
> Jtreg hotspot::hotspot_all_no_apps, jdk::jdk_core, langtools::tier1
> JCStress: tests-all
> 
> Thanks,
> Xiaohong Gong
> 
>