RFR: Fix Shenandoah build on s390

[Andrew Hughes]

On Thu, 23 Aug 2018 at 10:15, Aleksey Shipilev <shade at redhat.com> wrote:
>
> On 08/23/2018 05:37 AM, Andrew Hughes wrote:
> > Bug: https://icedtea.classpath.org/bugzilla/show_bug.cgi?id=3619
> > Webrev: http://cr.openjdk.java.net/~andrew/shenandoah-8/pr3619/
>
> Thanks!
>
> > A couple of build issues were found when trying to build the
> > Shenandoah source code base on s390. As the build failures in the
> > linked bug show, issues arise on s390 that are not visible on other
> > platforms, because size_t is not equal to uintx on that platform. On
> > s390, uintx is a unsigned int, while size_t is a long unsigned int.
> >
> > The attached patch fixes the build issues by switching two format
> > specifiers in the heuristics code from UINTX_FORMAT to SIZE_FORMAT,
>
> The block in shenandoahAdaptiveHeuristics is gone in our current development builds, and it will
> sift down to 8u as we next round of backporting next week. The block in shenandoahStaticHeuristics
> is not needed and can be removed:
>   http://hg.openjdk.java.net/shenandoah/jdk/rev/8cb652cfde2c

Works for me ;-)

>
> > and casting the region size from uintx to size_t in
> > shenandoahHeapRegion.cpp. The latter is necessary, as otherwise the
> > calls to MAX2 have mismatched types (uintx, size_t). A possible
> > alternative may be to use a size_t for the region size instead, but I
> > went with the solution that seemed the lowest risk on all platforms.
>
> Yes, that is a better option:
>   http://hg.openjdk.java.net/shenandoah/jdk/rev/1af5bd0d84eb

Yeah, that's what I tried on one attempt. I believe you'll also need
to change the
flag ShenandoahMinRegionSize if you want to get rid of both casts though, and
probably MaxRegionSize for consistency. That's why I backed down for the
initial patch.

>
> > The changes are applied against aarch64/shenandoah-jdk8u, as that's
> > where s390 builds were tested,
>
> So it would be easier for us to propagate the changes to shenandoah/jdk8, and then make a bulk
> integration with all these bugfixes into aarch64-port/shenandoah-jdk8u. We can put your change in
> aarch64-port/shenandoah-jdk8u as is, but that would show up as merge conflict later. So if there is
> no time pressure, I'd prefer to wait for bulk integration.
>

No pressure from me, I already have this locally patched in the RPM.

However, you're going to need more than the two changes you linked to.

> -Aleksey
>


-- 
Andrew :)

Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Web Site: http://fuseyism.com
Twitter: https://twitter.com/gnu_andrew_java
PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222