8203287: Zero fails to build after JDK-8199712 (Flight Recorder)

[Severin Gehwolf]

Hi David,

On Fri, 2018-05-18 at 14:17 +1000, David Holmes wrote:
> It doesn't look right to me that vm_version_ext_zero.cpp is zero 
> specific - it really should be CPU specific. The zero version is setting 
> up a bunch of dummy values for threads/cores/sockets etc, but if 
> something else uses them in calculations then you'd get some weird numbers.

Right. Do you expect this to be used outside JFR?

Note that Zero is supposed to be buildable on a wide range of
architectures. Basically all that have GCC and libffi. That would mean
we'd have to add CPU specific files for all such architectures.
Examples would be mips, ppc (32bit), s390 (31 bit) and others. It seems
more reasonable to add a zero specific version of it and produce dummy
values than requiring one to add CPU specific vm_version_ext files on
architectures where one happens to build Zero on just to get it to
build. What's more, it only seems to be used by JFR.

The trouble comes from os_perf_linux.cpp which got added in shared code
for JFR. I don't expect JFR to work on Zero, TBH. It seems not worth
having for Zero. But that's for later. Right now, the Zero build is
broken and we need to get it fixed. FWIW, I've tried this to disable
the jfr feature by default on Zero, but that still attempts to build
os_perf_linux.cpp:

diff --git a/make/autoconf/hotspot.m4 b/make/autoconf/hotspot.m4
--- a/make/autoconf/hotspot.m4
+++ b/make/autoconf/hotspot.m4
@@ -309,9 +309,11 @@
     AC_MSG_ERROR([Specified JVM feature 'cmsgc' requires feature 'serialgc'])
   fi
 
-  # Enable JFR by default, except on linux-sparcv9 and on minimal.
-  if test "x$OPENJDK_TARGET_OS" != xlinux || test "x$OPENJDK_TARGET_CPU" != xsparcv9; then
-    NON_MINIMAL_FEATURES="$NON_MINIMAL_FEATURES jfr"
+  # Enable JFR by default, except for Zero, linux-sparcv9 and on minimal.
+  if test "x$HOTSPOT_TARGET_CPU" != xzero; then
+    if test "x$OPENJDK_TARGET_OS" != xlinux || test "x$OPENJDK_TARGET_CPU" != xsparcv9; then
+      NON_MINIMAL_FEATURES="$NON_MINIMAL_FEATURES jfr"
+    fi
   fi
 
   # Turn on additional features based on other parts of configure

Thoughts?

Thanks,
Severin

> David
> 
> On 18/05/2018 12:38 AM, Severin Gehwolf wrote:
> > Hi Martin, Aleksey,
> > 
> > Ugh, sorry, my mistake :-/ Thanks for the reviews!
> > 
> > This is what I'm going to push once jdk-submit comes back clean:
> > 
> > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8203287/webrev.04/
> > 
> > Thanks,
> > Severin
> > 
> > On Thu, 2018-05-17 at 14:17 +0000, Doerr, Martin wrote:
> > > Hi Severin,
> > > 
> > > looks good to me with Aleksey's patch. Thanks for waiting.
> > > 
> > > Best regards,
> > > Martin
> > > 
> > > 
> > > -----Original Message-----
> > > From: Aleksey Shipilev [mailto:shade at redhat.com]
> > > Sent: Donnerstag, 17. Mai 2018 16:14
> > > To: Severin Gehwolf <sgehwolf at redhat.com>; Doerr, Martin <martin.doerr at sap.com>; hotspot-dev <hotspot-dev at openjdk.java.net>
> > > Subject: Re: 8203287: Zero fails to build after JDK-8199712 (Flight Recorder)
> > > 
> > > On 05/17/2018 04:02 PM, Severin Gehwolf wrote:
> > > > On Thu, 2018-05-17 at 15:27 +0200, Aleksey Shipilev wrote:
> > > > > On 05/17/2018 03:12 PM, Severin Gehwolf wrote:
> > > > > > Hi Martin, Aleksey,
> > > > > > 
> > > > > > Since JDK-8203288 has been pushed here is the latest webrev for Zero.
> > > > > > It no longer contains changes in os_perf_linux.cpp.
> > > > > > 
> > > > > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8203287/webrev.02/
> > > > > > 
> > > > > > How does it look?
> > > > > 
> > > > > Looks good, but I used parentheses inconsistently with the rest of Hotspot in the original patch:
> > > > > 
> > > > >   #if (defined X86 && !defined ZERO)
> > > > > 
> > > > > Should be:
> > > > > 
> > > > >   #if defined(X86) && !defined (ZERO)
> > > > 
> > > > OK. Here it goes:
> > > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8203287/webrev.03/
> > > 
> > > Notice the parentheses, it is done like that in other places:
> > > 
> > >   #if defined(X86) && !defined(ZERO)
> > > 
> > > Apply this patch to correct:
> > >    http://cr.openjdk.java.net/~shade/8203287/parent.patch
> > > 
> > > I checked it builds with x86_64 and Zero.
> > > 
> > > -Aleksey
> > >