[aarch64-port-dev ] RFR: 8205694: AArch64: Add test to validate volatile load, store and CAS code generation

[Zhongwei Yao]

Hi, Andrew,

Sorry for my separate comment, but I have a question about "storestore" barrier eliding in StoreCM node.

In http://cr.openjdk.java.net/~adinn/8205694/webrev.01/test/hotspot/jtreg/compiler/c2/aarch64/TestVolatiles.java.html line: 301, when "storestore" barrier is elided, is following sequence still ordered?:
  stlrw
  strb

To my knowledge, strb could be reordered in above sequence. But "stlrw; stlrb" will be ordered as following said.

And I see some discrepancy in the comment of aarch64.ad (without your patch applied, line: 1483 to 1484), storeCM is translated to:
  dmb ishst
  strlb

But actually, it is translated to:
  dmb ishst
  strb

So either the comment should be updated, or the code generated is not correct.

--
Best regards,
Zhongwei

________________________________________
From: aarch64-port-dev <aarch64-port-dev-bounces at openjdk.java.net> on behalf of Dmitrij Pochepko <dmitrij.pochepko at bell-sw.com>
Sent: Thursday, June 28, 2018 12:40:50 AM
To: Andrew Dinn
Cc: hotspot compiler; aarch64-port-dev at openjdk.java.net
Subject: Re: [aarch64-port-dev ] RFR: 8205694: AArch64: Add test to validate volatile load, store and CAS code generation

Hi,

looks good to me(with @run main changed to @run driver)

Thanks,
Dmitrij

27.06.2018 20:28, Andrew Dinn wrote:
> Here is an updated webrev which addresses all comments modulo Dmitrij's
> recommendation about use of @run driver for which I am still awaiting
> clarification. I hope the changes described below are satisfactory.
>
>     webrev: http://cr.openjdk.java.net/~adinn/8205694/webrev.01
>     JIRA: https://bugs.openjdk.java.net/browse/JDK-8205694
>
> Dmitirj, if you can explain what else I need to tweak then I'll be happy
> to update the webrev. If not please let me know if it is ok to push.
>
> Changes:
>
> Modified asserts in the aarch64.ad predicates to guarantees in all cases
> where they relate to expectations about the graph structure. The ones
> that are still left as asserts refer to expectations about what nodes
> should be passed as predicate arguments i.e. the former are about
> consistency external to the aarch64.ad file and the latter to internal
> consistency (I don't believe the latter merit a check in product code).
>
> Added copyright headers to all test files
>
> Turned the original test class (TestVolatiles) into a test runner which
> can be used to check a specific volatile op + GC config combination.
>
> Added 5 new test classes (TestVolatilesG1, TestVolatilesSerial,
> TestVolatilesParallel, TestVolatilesCMS, and TestVolatilesCMSCardMark)
> which indirect control to TestVolatiles to test each of the 5 different
> volatile ops with one of the relevant GC configurations.
>
> Added @require conditions to each of the 5 new tests to ensure it is run
> in a server GC and that the GC confguration under test is supported by
> the JVM.
>
> Testing:
>
> All 5 tests passed on a debug, server VM.
>
> 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, Eric Shander
>