> On Mar 14, 2016, at 3:17 PM, Andrew Hughes <gnu.and...@redhat.com> wrote: > > Bug: https://bugs.openjdk.java.net/browse/JDK-8151841 > Webrev: http://cr.openjdk.java.net/~andrew/8151841/webrev.01/ > > A number of additional flags need to be passed to the C and C++ compiler > for OpenJDK to compile with GCC 6:
This might not be the best time to be making interesting changes to the Hotspot build system in order to support a not yet released compiler, since there is a new Hotspot build system coming soon: https://bugs.openjdk.java.net/browse/JDK-8076052, http://openjdk.java.net/jeps/284. But I'll leave that up to the folks in charge of the build infrastructure. That said, here are some specific comments on the webrev. ------------------------------------------------------------------------------ common/autoconf/hotspot-spec.gmk.in Needs copyright update. ------------------------------------------------------------------------------ common/autoconf/flags.m4 631 # These flags are required for GCC 6 builds but may not be available on earlier versions 632 NO_NULL_POINTER_CHECK_CFLAG="-fno-delete-null-pointer-checks" According to gcc documentation, this option goes back at least into the 3.x series. So this seems somewhat overkill. ------------------------------------------------------------------------------ common/autoconf/flags.m4 631 # These flags are required for GCC 6 builds but may not be available on earlier versions ... 636 NO_LIFETIME_DSE_CFLAG="-fno-lifetime-dse" This one does seem to be relatively new; I think it's introduced in gcc4.9. However, there are other places where version conditionalization of options is performed, such as hotspot/make/linux/makefiles/gcc.make, where the addition of some options is dependent on the version. Here it's done based on behavior rather than simply acceptance by the compiler being used. So, for example, -Wuninitialized isn't turned on until gcc4.8, because earlier versions apparently report false positives. Of course, there's the annoying fact that there are multiple gcc.make files that might need to be modified for this. But I'm not sure the simple "the compiler accepts this option" is the right test here. ------------------------------------------------------------------------------ common/autoconf/flags.m4 588 elif test "x$TOOLCHAIN_TYPE" = xgcc; then 589 CXXSTD_CXXFLAG="-std=gnu++98" 590 FLAGS_CXX_COMPILER_CHECK_ARGUMENTS(ARGUMENT: [$CXXSTD_CXXFLAG -Werror], 591 IF_FALSE: [CXXSTD_CXXFLAG=""]) Checking for acceptance of this option seems like overkill. ------------------------------------------------------------------------------ common/autoconf/flags.m4 If the new option checks weren't being made (as discussed above), the changes to the checker wouldn't be needed. ------------------------------------------------------------------------------ common/autoconf/hotspot-spec.gmk.in 113 EXTRA_CFLAGS=@LEGACY_EXTRA_CFLAGS@ $(CFLAGS_CCACHE) $(NO_NULL_POINTER_CHECK_FLAG) $(NO_LIFETIME_DSE_CFLAG) $(CXXSTD_CXXFLAG) It seems strange to only include $(NO_NULL_POINTER_CHECK_FLAG) and $(NO_LIFETIME_DSE_FLAG) in EXTRA_CFLAGS, but not EXTRA_CXXFLAGS. And it seems strange to include $(CXXSTD_CXXFLAG) in EXTRA_CFLAGS at all, rather than in EXTRA_CXXFLAGS. ------------------------------------------------------------------------------ > 2. A number of optimisations in GCC 6 lead to a broken JVM. We need to > add -fno-delete-null-pointer-checks and -fno-lifetime-dse to get a > working JVM. That's very disturbing! -fdelete-null-pointer-checks is the default in much earlier versions than gcc6 (since 4.5?), and much longer than that at higher optimization levels (since somewhere in the 3.x series?). But maybe gcc6 is doing a better job of recognizing the relevant situations. Or maybe there's a bug in gcc6, which is not a released version yet. One specific gcc6 change that could be relevant is: Value range propagation now assumes that the this pointer of C++ member functions is non-null. This eliminates common null pointer checks but also breaks some non-conforming code-bases (such as Qt-5, Chromium, KDevelop). As a temporary work-around -fno-delete-null-pointer-checks can be used. Wrong code can be identified by using -fsanitize=undefined. There's also a new warning option in gcc6 that might help find places where -fdelete-null-pointer-checks is leading to problems: -Wnull-dereference. Do you have any information as to where things are being broken by this option? I think I would prefer an attempt to track down this class of problem rather than hiding it via -fno-delete-null-pointer-checks. I don't have any suggestions for why gcc6 might be having problems because of -flifetime-dse, or how to find them. Do you? This seems to be a relatively new option, having been introduced in gcc4.9(?), and seems to have always been on by default since being introduced. Again, this could be a matter of gcc6 doing a better job of recognizing relevant situations, or a bug in that not-yet-released version.
