RFR: JDK-8198724 Refactor FLAGS handling in configure (original) (raw)

Erik Joelsson erik.joelsson at oracle.com
Thu Mar 1 23:46:09 UTC 2018

Hello Magnus,

Nice to finally see this posted! Overall very nice improvement.

On 2018-02-26 15:40, Magnus Ihse Bursie wrote:

The current way we're handling FLAGS (CFLAGS/CXXFLAGS, LDFLAGS, etc) is laden with technical debt. It's a sorry mess, hard to do anything constructive about.

This is the first step towards a better solution. The main goal is to have a set of more specific flags, more like how the SYSROOT flags work today, that can be individually included, instead of a huge CFLAGSJDKLIB. To make such a change manageable, this first step will (almost) only refactor the autoconf code, but will output the same flags in the spec.gmk. To minimize special cases, I have accepted the following differences: * On linux/gcc, we will get a -m, e.g. -m64 always (as on aix/solaris). * On macosx/clang, we will get a -m, e.g. -m64 always (as on aix/solaris). * On macosx/clang, we will now "hardcode" the max/min version number without dots. I would advocate a simple bash replace to remove the dots, like this: MACOSX_VERSION_MIN_NODOTS=${MACOSX_VERSION_MIN//./} * On macosx/clang, the JVMCFLAGS for the build toolchain will now also get an -fPIC (this was mysteriously missing before). * On windows/microsoft, the build toolchain will now get -nologo added as well, and also -homeparams for debug builds. * On solaris/solstudio, the JDKEXE flags will now include -errfmt, and CFLAGSJDKEXE will include -errtags=yes. The duplication of -errtags=yes in CXXFLAGSJDKLIB is removed. * On solaris/solstudio, we now always use -KPIC as pic flag. (Previously, we used -xcode=pic32 on sparc, but this is equivalent to -KPIC, so there's no reason for a special case.) * On solaris/solstudio, we now use e.g. "-Wc,-Qrm-s" instead of "-Qoption cg -Qrm-s" for C++ as well as for C code. The two formats are equivalent (for passing options to a certain compilation phase), and there was no reason to use different ones for C and C++. And for clarity, I have also renamed some exported flags to clearly show that they are consumed by LIBJSIG and ADLC. I have also renamed "/STACK" on the Microsoft linker to "-stack", to match how we write options elsewhere. I believe none of these should have any real effect, but if anything, they could probably be considered bug fixes. I have considered whitespace and ordering differences as irrelevant. In some cases ordering may be relevant, hopefully the COMPARE_BUILD run will uncover if that's the case. If those are ok, then I'm basically happy with this transformation. I have manually verified that the generated spec.gmk and buildjdk-spec.gmk matches this (no changes apart from the one listed above) for linux/x64/gcc, solaris/sparcv9/solstudio, windows/x64/microsoft and macosx/x64/clang, for release and fastdebug. I am also currently running a test job using the COMPAREBUILD functionality on our internal build system. I would appreciate if comments are more in the form of "think of this for subsequent updates to the flag handling" rather than requests to change this patch, at least for requests that are just not small fixes. (I've been juggling this for a while, trying to get it right...) Bug: https://bugs.openjdk.java.net/browse/JDK-8198724 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8198724-refactor-flags-step-1/webrev.01 Comments on things I saw, not necessarily needing fixes now:

In flags.m4, MACHINE_FLAG and COMPILER_TARGET_BITS_FLAG are redundant without comment.

flags-cflags.m4, FLAGS_SETUP_SHARED_LIBS, comments out of date/irrelevant like:

Default works for linux, might work on other platforms as well.

Solaris -errtags is not needed in CFLAGS_WARNINGS_ARE_ERRORS.

Given that TOOLCHAIN_MINIMUM_VERSION_gcc="4.7", perhaps we can remove the check for -Wno-X on gcc 4.4.

/Erik

/Magnus

More information about the build-dev mailing list