[aarch64-port-dev ] RFR: AArch64: incorrect code generation for StoreCM

[Zhongwei Yao]

Hi, Andrew,

It looks good to me.

>From: Andrew Dinn <adinn at redhat.com>
>Sent: Wednesday, July 4, 2018 5:31 PM
>To: Zhongwei Yao; Andrew Haley
>Cc: aarch64-port-dev at openjdk.java.net; hotspot compiler; nd
>Subject: Re: [aarch64-port-dev ] RFR: AArch64: incorrect code generation for StoreCM
>
>Hi Zhongwei,
>
>Thank you for your review.
>
>On 04/07/18 08:17, Zhongwei Yao wrote:
>> I find a typo in the TestVolatiles.java file: it uses
>> "CMSCondCardMark" in checkstore() and checkcas() but the testType is
>> past as "CMSCondMark". It leads to "CMSCondCardMark" case not tested.
>
>Yes, indeed. Thanks for catching that.
>
>> And the condition in aarch64.ad:2871 (patched)
>>   UseConcMarkSweepGC || !UseCondCardMark
>> should be "UseConcMarkSweepGC && !UseCondCardMark", right? (It could
>> be catched if the previous typo is fixed).
>
>Doh, once again thanks. The current heat wave in the UK is clearly
>making my logic circuits execute some invalid boolean reductions.

The memory model related code needs a lot of effort to make it right. Your work is really impressive!

>
>Applying the first fix to the test code led to a failure for
>TestVolatilesCMSCondMark as you described.
>
>Applying the second fix to aarch64.ad restores all tests to working
>correctly.
>
>An updated webrev against the latest jdk11 is available here
>
>  http://cr.openjdk.java.net/~adinn/8206163/webrev.01/
>
>> I'm thinking whether it is OK to replace "stlr; dmb ishst; strb" with
>> "stlr; strlb" when we using CMS without conditional card marking.
>> What do you think?
>
>Yes, that does look better. However, I would prefer to implement that as
>a separate fix if that is ok. I'd like to have a correct version checked
>in before we try to implement an optimized version.
>
Yeah, agree. 

--
Best regards,
Zhongwei