PR3634: Shenandoah still broken on s390 with aarch64-shenandoah-jdk8u181-b16

[Aleksey Shipilev]

On 11/12/2018 05:34 PM, Andrew Hughes wrote:
> The build of Shenandoah is still broken on s390 (the 31-bit platform)
> after the latest merge, due to a mismatch in the MAX2 macro:
> 
> https://icedtea.classpath.org/bugzilla/show_bug.cgi?id=3634
> 
> The working solution I have for this at present (and used in our
> latest RPMs & IcedTea release) is to update ShenandoahMinRegionSize
> and ShenandoahMaxRegionSize to be size_t instead of uintx. This
> shouldn't make a difference on other platforms where they are the
> same. It does on s390 where size_t is bigger than uintx.
> 
> https://cr.openjdk.java.net/~andrew/shenandoah-8/pr3634/webrev.01/

I think this would not work without:
  https://bugs.openjdk.java.net/browse/JDK-8054823

If you run Shenandoah tests with patch right now, then gc/shenandoah/options/TestRegionSizeArgs.java
would fail with "Improperly specified VM option 'ShenandoahMinRegionSize=255K"

In other words, we have to keep it uintx, and cast where appropriate. For example, like this:

# HG changeset patch
# User shade
# Date 1542042122 -3600
#      Mon Nov 12 18:02:02 2018 +0100
# Node ID d215450bcdcad401497f2269f9448104ac483b29
# Parent  bffbdb3f8f1354f92a16913ee55ce8d507ce8474
Cast Shenandoah{Min,Max}RegionSize to size_t

diff -r bffbdb3f8f13 -r d215450bcdca src/share/vm/gc_implementation/shenandoah/shenandoahHeapRegion.cpp
--- a/src/share/vm/gc_implementation/shenandoah/shenandoahHeapRegion.cpp        Sun Nov 11 13:20:44
2018 +0100
+++ b/src/share/vm/gc_implementation/shenandoah/shenandoahHeapRegion.cpp        Mon Nov 12 18:02:02
2018 +0100
@@ -508,8 +508,8 @@
     region_size = max_heap_size / ShenandoahTargetNumRegions;

     // Now make sure that we don't go over or under our limits.
-    region_size = MAX2(ShenandoahMinRegionSize, region_size);
-    region_size = MIN2(ShenandoahMaxRegionSize, region_size);
+    region_size = MAX2<size_t>(ShenandoahMinRegionSize, region_size);
+    region_size = MIN2<size_t>(ShenandoahMaxRegionSize, region_size);

   } else {
     if (ShenandoahHeapRegionSize > initial_heap_size / MIN_NUM_REGIONS) {

Can you try if this helps s390? If so, we would push it to sh/jdk8u, and it can then be either
cherry-picked to integration repo, or come with the next bulk integration.

-Aleksey