RFR: JDK-8198724 Refactor FLAGS handling in configure

[Magnus Ihse Bursie]

On 2018-03-02 00:46, Erik Joelsson wrote:
> Hello Magnus,
>
> Nice to finally see this posted! Overall very nice improvement.
Thank you.

> I would advocate a simple bash replace to remove the dots, like this:
> MACOSX_VERSION_MIN_NODOTS=${MACOSX_VERSION_MIN//\./}

Ok, I updated this. I see no need to post another review for this.

>> * On macosx/clang, the JVM_CFLAGS 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 CFLAGS_JDKEXE will include -errtags=yes. The duplication of 
>> -errtags=yes in CXXFLAGS_JDKLIB 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.
COMPARE_BUILD runs on our internal mach5 system showed no discrepancies. 
(This was the reason I fixed the COMPARE_BUILD system for no-change builds.)

I actually don't know of any cases where CFLAGS ordering is relevant. 
(LIBS is another issue) It's good to be aware of such issues, so please 
enlighten me. :-)

I noticed one thing, though, was that a trivial sort of the flags before 
writing them to spec.gmk didn't work. I did this (and similarly on a 
patched baseline) to facilitate my comparison, but it was not 
compilable. The reason was that a few flags (actually, currently only on 
clang/macosx) had arguments that was space separated from the options. 
For most flags, we do not have this scenario, which I think is good. It 
makes it easier to see what is a "single" flag.

>> 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 COMPARE_BUILD 
>> 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.
Oh, how I wish this was true. :-( COMPILER_TARGET_BITS_FLAG is actually 
exported for use in (you guess!) GensrcX11Wrappers.gmk. (Guess why I was 
targeting that for cleanup :-)).

When the GensrcX11Wrappers fix is in, I intend to clean up this.

>
> flags-cflags.m4, FLAGS_SETUP_SHARED_LIBS, comments out of 
> date/irrelevant like:
> # Default works for linux, might work on other platforms as well.
I'll go through those at a later date. For the moment, 
FLAGS_SETUP_SHARED_LIBS was "good enough" to be left alone.

> Solaris -errtags is not needed in CFLAGS_WARNINGS_ARE_ERRORS.
Yes. I'm currently working on a follow-up patch were I redesign the 
warnings handling, including -errtags.
> Given that TOOLCHAIN_MINIMUM_VERSION_gcc="4.7", perhaps we can remove 
> the check for -Wno-X on gcc 4.4.

Good find. It would be nice to get rid of it.

In fact, I wonder if it is possible to raise the minimum gcc version to 
4.8 for JDK11. That would help us get rid of even more gcc version 
checks for broken/bad behaviour in pre-4.8 gcc.

As you can see, there's a lot of follow-up cleaning left to do. This 
refactoring was scary enough that I wanted to minimize the changes done 
in this first step.

/Magnus

>
> /Erik
>
>> /Magnus
>>
>