Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

2016-07-11 Thread David Holmes 

On 12/07/2016 1:40 PM, Andrew Hughes wrote:

- Original Message -

Catching up ...

On 11/07/2016 7:05 AM, Andrew Hughes wrote:



- Original Message -

On Jul 8, 2016, at 2:38 AM, Erik Joelsson 
wrote:

Hello,

This looks good except for the change in toolchain.m4, which looks like
it
might actually break cross compilation by overriding the value for
compiler version for the build compiler using the target compiler. With
this change we basically have:

if cross compilation
 TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS([BUILD_], [OPENJDK_BUILD_])
else
 ...
fi
TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS([], [OPENJDK_BUILD_])

The problem you are trying to solve is that in the case of not cross
compilation, the TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS macro wasn't
called with "OPENJDK_BUILD_". Kim's suggested patch was to add the call
in
the else clause.


Good catch!  I totally missed that when reviewing.



Yes, good catch! The indenting on the bug report must have confused me.

Fixed version:

http://cr.openjdk.java.net/~andrew/8156980/webrev.06/root/
http://cr.openjdk.java.net/~andrew/8156980/webrev.06/hotspot


I was glad to see you realized that parts of the hotspot build (libjsig,
libdtrace) only deal with C programs not C++ and so don't want and
should not have a C++ setting passed. But that then begs the question in
the main build in flags.m4 why we have:

  $2JVM_CFLAGS="${$2JVM_CFLAGS} ${$2CXXSTD_CXXFLAG}"

which again adds a C++ related flags to what should be the C compilation
flags ??



You'd think so, but for some reason, the HotSpot build has always called the
C++ compiler flags 'CFLAGS'. There is no JVM_CXXFLAGS.


Ah now I recall - this is the CPP -> CXX mess. CFLAGS meant 
compiler-flags; CPPFLAGS meant pre-processor-flags. People thought CPP 
meant C++ and changed to CXX.



The same behaviour was true with the old build; we had to pass these options
down to HotSpot in EXTRA_CFLAGS.

JVM_CFLAGS is the main variable used in the HotSpot build (see 
make/lib/CompileJvm.gmk).
The only C files in the HotSpot tree are for libsaproc, jsig and DTrace,
all of which are handled by separate makefiles. The only one to use JVM_CFLAGS 
is
make/lib/CompileDtracePostJvm.gmk, where it is used for a single C++ file, 
generateJvmOffsets.cpp:

generateJvmOffsets.cpp_CXXFLAGS := $(JVM_CFLAGS) -mt -xnolib 
-norunpath, \
generateJvmOffsetsMain.c_CFLAGS := -library=%none -mt -m64 -norunpath 
-z nodefs, \

Thus, it seems JVM_CFLAGS is JVM_CXXFLAGS in all but name.


Thanks for stepping through that.

David


If -std=gnu++98 does get passed to the C compiler, it produces a warning:

cc1: warning: command line option '-std=gnu++98' is valid for C++/ObjC++ but 
not for C

I don't see this at all for this patch on OpenJDK 9. It does appear in the 
backport
to 8u [0], because the aforementioned C code in the old HotSpot build does use 
the
same CFLAGS there.


Thanks,
David
-


HotSpot webrev is unchanged.

Thanks,





[0] http://mail.openjdk.java.net/pipermail/jdk8u-dev/2016-July/005690.html

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

2016-07-11 Thread Andrew Hughes 
- Original Message -
> Catching up ...
> 
> On 11/07/2016 7:05 AM, Andrew Hughes wrote:
> >
> >
> > - Original Message -
> >>> On Jul 8, 2016, at 2:38 AM, Erik Joelsson 
> >>> wrote:
> >>>
> >>> Hello,
> >>>
> >>> This looks good except for the change in toolchain.m4, which looks like
> >>> it
> >>> might actually break cross compilation by overriding the value for
> >>> compiler version for the build compiler using the target compiler. With
> >>> this change we basically have:
> >>>
> >>> if cross compilation
> >>>  TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS([BUILD_], [OPENJDK_BUILD_])
> >>> else
> >>>  ...
> >>> fi
> >>> TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS([], [OPENJDK_BUILD_])
> >>>
> >>> The problem you are trying to solve is that in the case of not cross
> >>> compilation, the TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS macro wasn't
> >>> called with "OPENJDK_BUILD_". Kim's suggested patch was to add the call
> >>> in
> >>> the else clause.
> >>
> >> Good catch!  I totally missed that when reviewing.
> >>
> >>
> > Yes, good catch! The indenting on the bug report must have confused me.
> >
> > Fixed version:
> >
> > http://cr.openjdk.java.net/~andrew/8156980/webrev.06/root/
> > http://cr.openjdk.java.net/~andrew/8156980/webrev.06/hotspot
> 
> I was glad to see you realized that parts of the hotspot build (libjsig,
> libdtrace) only deal with C programs not C++ and so don't want and
> should not have a C++ setting passed. But that then begs the question in
> the main build in flags.m4 why we have:
> 
>   $2JVM_CFLAGS="${$2JVM_CFLAGS} ${$2CXXSTD_CXXFLAG}"
> 
> which again adds a C++ related flags to what should be the C compilation
> flags ??
> 

You'd think so, but for some reason, the HotSpot build has always called the
C++ compiler flags 'CFLAGS'. There is no JVM_CXXFLAGS.

The same behaviour was true with the old build; we had to pass these options
down to HotSpot in EXTRA_CFLAGS.

JVM_CFLAGS is the main variable used in the HotSpot build (see 
make/lib/CompileJvm.gmk).
The only C files in the HotSpot tree are for libsaproc, jsig and DTrace,
all of which are handled by separate makefiles. The only one to use JVM_CFLAGS 
is
make/lib/CompileDtracePostJvm.gmk, where it is used for a single C++ file, 
generateJvmOffsets.cpp:

generateJvmOffsets.cpp_CXXFLAGS := $(JVM_CFLAGS) -mt -xnolib 
-norunpath, \
generateJvmOffsetsMain.c_CFLAGS := -library=%none -mt -m64 -norunpath 
-z nodefs, \

Thus, it seems JVM_CFLAGS is JVM_CXXFLAGS in all but name.

If -std=gnu++98 does get passed to the C compiler, it produces a warning:

cc1: warning: command line option '-std=gnu++98' is valid for C++/ObjC++ but 
not for C

I don't see this at all for this patch on OpenJDK 9. It does appear in the 
backport
to 8u [0], because the aforementioned C code in the old HotSpot build does use 
the
same CFLAGS there.

> Thanks,
> David
> -
> 
> > HotSpot webrev is unchanged.
> >
> > Thanks,
> >
> 

[0] http://mail.openjdk.java.net/pipermail/jdk8u-dev/2016-July/005690.html
-- 
Andrew :)

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

PGP Key: ed25519/35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222

Re: Enable hotspot builds for 4.x Linux kernels.

2016-07-11 Thread David Holmes 

Hi Jeremy,

On 11/07/2016 6:10 PM, Jeremy Whiting wrote:

Hi,

 Last week I started the process to build jdk8 using a Fedora 23
environment. Followed the detailed instructions but came across an
issue. Looking at the bugs database identical issues have been raised
for various components [1] and jdk versions. It appears the backport
missed it into the hotspot repo for jdk8. Browsing the hotspot repo
shows [2] the supported os version doesn't include a 4.x Linux kernel.

 Can this fix be backported to the jdk8 hotspot repo as well ?


Sure. See:

http://openjdk.java.net/projects/jdk8u/approval-template.html

and

http://openjdk.java.net/projects/jdk8u/groundrules.html

Any 8u committer can do this.

Cheers,
David



$ hg diff
diff -r 87ee5ee27509 make/linux/Makefile
--- a/make/linux/MakefileTue Mar 04 11:51:03 2014 -0800
+++ b/make/linux/MakefileMon Jul 11 08:59:44 2016 +0100
@@ -225,7 +225,7 @@
 # Solaris 2.5.1, 2.6).
 # Disable this check by setting DISABLE_HOTSPOT_OS_VERSION_CHECK=ok.

-SUPPORTED_OS_VERSION = 2.4% 2.5% 2.6% 3%
+SUPPORTED_OS_VERSION = 2.4% 2.5% 2.6% 3% 4%
 OS_VERSION := $(shell uname -r)
 EMPTY_IF_NOT_SUPPORTED = $(filter $(SUPPORTED_OS_VERSION),$(OS_VERSION))

$


Regards,

Jeremy


$ uname -a
Linux burtha-f23 4.5.7-200.fc23.x86_64 #1 SMP Wed Jun 8 17:41:50 UTC
2016 x86_64 x86_64 x86_64 GNU/Linux

[1]
https://bugs.openjdk.java.net/browse/JDK-8087178?jql=text%20~%20%22Enable%20hotspot%20builds%20on%204.x%20Linux%20kernels%22


[2]
http://hg.openjdk.java.net/jdk8/jdk8/hotspot/file/87ee5ee27509/make/linux/Makefile#l228

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

2016-07-11 Thread David Holmes 

Catching up ...

On 11/07/2016 7:05 AM, Andrew Hughes wrote:



- Original Message -

On Jul 8, 2016, at 2:38 AM, Erik Joelsson  wrote:

Hello,

This looks good except for the change in toolchain.m4, which looks like it
might actually break cross compilation by overriding the value for
compiler version for the build compiler using the target compiler. With
this change we basically have:

if cross compilation
 TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS([BUILD_], [OPENJDK_BUILD_])
else
 ...
fi
TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS([], [OPENJDK_BUILD_])

The problem you are trying to solve is that in the case of not cross
compilation, the TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS macro wasn't
called with "OPENJDK_BUILD_". Kim's suggested patch was to add the call in
the else clause.


Good catch!  I totally missed that when reviewing.



Yes, good catch! The indenting on the bug report must have confused me.

Fixed version:

http://cr.openjdk.java.net/~andrew/8156980/webrev.06/root/
http://cr.openjdk.java.net/~andrew/8156980/webrev.06/hotspot


I was glad to see you realized that parts of the hotspot build (libjsig, 
libdtrace) only deal with C programs not C++ and so don't want and 
should not have a C++ setting passed. But that then begs the question in 
the main build in flags.m4 why we have:


 $2JVM_CFLAGS="${$2JVM_CFLAGS} ${$2CXXSTD_CXXFLAG}"

which again adds a C++ related flags to what should be the C compilation 
flags ??


Thanks,
David
-


HotSpot webrev is unchanged.

Thanks,

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

2016-07-11 Thread Kim Barrett 
> On Jul 10, 2016, at 5:05 PM, Andrew Hughes  wrote:
> 
> 
> 
> - Original Message -
>>> On Jul 8, 2016, at 2:38 AM, Erik Joelsson  wrote:
>>> 
>>> Hello,
>>> 
>>> This looks good except for the change in toolchain.m4, which looks like it
>>> might actually break cross compilation by overriding the value for
>>> compiler version for the build compiler using the target compiler. With
>>> this change we basically have:
>>> 
>>> if cross compilation
>>> TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS([BUILD_], [OPENJDK_BUILD_])
>>> else
>>> ...
>>> fi
>>> TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS([], [OPENJDK_BUILD_])
>>> 
>>> The problem you are trying to solve is that in the case of not cross
>>> compilation, the TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS macro wasn't
>>> called with "OPENJDK_BUILD_". Kim's suggested patch was to add the call in
>>> the else clause.
>> 
>> Good catch!  I totally missed that when reviewing.
>> 
>> 
> Yes, good catch! The indenting on the bug report must have confused me.
> 
> Fixed version:
> 
> http://cr.openjdk.java.net/~andrew/8156980/webrev.06/root/
> http://cr.openjdk.java.net/~andrew/8156980/webrev.06/hotspot
> 
> HotSpot webrev is unchanged.

Looks good.

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

2016-07-11 Thread Omair Majid 
* Andrew Hughes  [2016-07-07 11:53]:
> Sadly, I celebrated too soon; it seems that change just delayed the failure
> until much later in the build.
> 
> I've found the problem though. In 
> src/share/vm/utilities/globalDefinitions.hpp,
> we have:
> 
> #ifdef max
> #undef max
> #endif
> 
> #ifdef min
> #undef min
> #endif
> 
> #define max(a,b) Do_not_use_max_use_MAX2_instead
> #define min(a,b) Do_not_use_min_use_MIN2_instead
> 
> The problem is that max and min are now longer macros in the GCC 6 C++ 
> library.
> So they can't be undefined.
> 
> The build succeeds if I disable (#if 0) the definition of max and min. So
> we need to consider either removing them or _GLIBCXX_INCLUDE_NEXT_C_HEADERS
> needs to be defined to get the old behaviour. I prefer the former.

I ran into this too and I posted a separate patch to work around this
issue that just involves changing the ordering of the includes:

http://mail.openjdk.java.net/pipermail/hotspot-dev/2016-July/023910.html

Thanks,
Omair

-- 
PGP Key: 66484681 (http://pgp.mit.edu/)
Fingerprint = F072 555B 0A17 3957 4E95  0056 F286 F14F 6648 4681

Enable hotspot builds for 4.x Linux kernels.

2016-07-11 Thread Jeremy Whiting 

Hi,

 Last week I started the process to build jdk8 using a Fedora 23 
environment. Followed the detailed instructions but came across an 
issue. Looking at the bugs database identical issues have been raised 
for various components [1] and jdk versions. It appears the backport 
missed it into the hotspot repo for jdk8. Browsing the hotspot repo 
shows [2] the supported os version doesn't include a 4.x Linux kernel.


 Can this fix be backported to the jdk8 hotspot repo as well ?


$ hg diff
diff -r 87ee5ee27509 make/linux/Makefile
--- a/make/linux/MakefileTue Mar 04 11:51:03 2014 -0800
+++ b/make/linux/MakefileMon Jul 11 08:59:44 2016 +0100
@@ -225,7 +225,7 @@
 # Solaris 2.5.1, 2.6).
 # Disable this check by setting DISABLE_HOTSPOT_OS_VERSION_CHECK=ok.

-SUPPORTED_OS_VERSION = 2.4% 2.5% 2.6% 3%
+SUPPORTED_OS_VERSION = 2.4% 2.5% 2.6% 3% 4%
 OS_VERSION := $(shell uname -r)
 EMPTY_IF_NOT_SUPPORTED = $(filter $(SUPPORTED_OS_VERSION),$(OS_VERSION))

$


Regards,

Jeremy


$ uname -a
Linux burtha-f23 4.5.7-200.fc23.x86_64 #1 SMP Wed Jun 8 17:41:50 UTC 
2016 x86_64 x86_64 x86_64 GNU/Linux


[1] 
https://bugs.openjdk.java.net/browse/JDK-8087178?jql=text%20~%20%22Enable%20hotspot%20builds%20on%204.x%20Linux%20kernels%22


[2] 
http://hg.openjdk.java.net/jdk8/jdk8/hotspot/file/87ee5ee27509/make/linux/Makefile#l228




--
Jeremy Whiting
Senior Software Engineer, JBoss Performance Team
Red Hat


Registered Address: RED HAT UK LIMITED, Peninsular House, 30-36 Monument 
Street, 4th floor, London. EC3R 8NB United Kingdom
Registered in UK and Wales under Company Registration No. 3798903  Directors: Michael 
Cunningham (US), Michael ("Mike") O'Neill (Ireland) and Eric Shander (US)