[aarch64-port-dev ] [EXT] Re: [Roland Westrelin] Re: Aarch64 port for ZGC, so far

[Derek White]

Hi Stuart,

That's great news!

My vote is for separate patches where possible.  It will make pinpointing any potential bugs in the checkins easier, as well as reviewing.

Actually, I hate to do this, but I'll go further...

I think that the oop64 patch could be broken up into patch (A) that adds the patch_info_offset() method, and patch (B) that supports oop64 on aarch64.

Patch (A) is a small fix across many files, and if it's correct(*), it's Correct by Trivial Inspection (CBTI). Could be checked in soon with the understanding that this is required for upcoming patch.

Patch (B) requires some deeper thought, and reviewing is a bit easier with a smaller file set to track. And it should be (mostly) in aarch64 code, not shared.

I'm not sure if the compiler folks need this level of factoring, but it is quite common in the GC team.

 - Derek

(*) I went looking for a caller of new patch_info_offset() method and didn't find one across any of the patches. It looks like it was used in an earlier version of the patch?

> -----Original Message-----
> From: aarch64-port-dev <aarch64-port-dev-bounces at openjdk.java.net> On
> Behalf Of Stuart Monteith
> Sent: Thursday, March 14, 2019 1:26 PM
> To: Andrew Haley <aph at redhat.com>; Roland Westrelin
> <rwestrel at redhat.com>
> Cc: zgc-dev at openjdk.java.net; aarch64-port-dev <aarch64-port-
> dev at openjdk.java.net>
> Subject: [EXT] Re: [aarch64-port-dev ] [Roland Westrelin] Re: Aarch64 port
> for ZGC, so far
> 
> External Email
> 
> ----------------------------------------------------------------------
> Hello,
>    Thanks to Andrew, C1 is now working. I've updated the patches to as they
> are today.
> 
> The patches are here:
> 
> http://cr.openjdk.java.net/~smonteith/zgc/20190314/
> 
> I'm doing some more testing and then move onto RFRs.
> 
> I have a choice of rolling Roland's membar patch into the ZGC patch and
> adding him as a contributor, or I can raise a separate Java bug and submit it
> as a separate patch. Are there preferences?
> 
> BR,
>    Stuart
> 
> On Fri, 8 Mar 2019 at 16:34, Stuart Monteith <stuart.monteith at linaro.org>
> wrote:
> >
> > Hi,
> >    I find when running with -XX:TieredStopAtLevel=1 and ZGC enabled it
> > fails when running with Lucene's indexing demo
> > "org.apache.lucene.demo.IndexFiles" or SPECjbb2015.
> > I tried javac, and other simple examples, but I haven't managed to
> > find anything simple that provokes the issue.
> >
> > I've uploaded this here:
> >   http://cr.openjdk.java.net/~smonteith/zgc/Lucene-demo.tar.xz
> >
> > If you have the appropriate java command on your patch, if you run
> > "index.sh" with a directory of files index as the parameter, it will
> > reproduce the problem.
> >
> >
> > BR,
> >    Stuart
> >
> > On Fri, 8 Mar 2019 at 16:19, Andrew Haley <aph at redhat.com> wrote:
> > >
> > > On 3/7/19 11:36 AM, Stuart Monteith wrote:
> > > > There is an issue with C1 - the changes in "8217717: ZGC: Broken
> > > > oop map in C1 load barrier stub" did regress things somewhat
> > > > causing crashes with stale references. However, As patching on
> > > > aarch64 C1 isn't really supported, I'm not satisfied it was
> > > > working correctly before. Patching lea may impractical as things stand
> today.
> > >
> > > I can't get it to fail. Please tell me exactly what I have to do to
> > > demonstrate the C1 failure.
> > >
> > > --
> > > Andrew Haley
> > > Java Platform Lead Engineer
> > > Red Hat UK Ltd. <https://www.redhat.com>
> > > EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671