[OpenJDK 2D-Dev] JDK 9 build with GCC 6.1.1

[Phil Race]

On 06/27/2016 08:50 PM, Yasumasa Suenaga wrote:
> Hi Kim,
>
> The newest changes for jdk repos is [1].
> Erik points we should use DISABLED_WARNINGS_gcc to handle unknown 
> warning tags. [2]
> [1] is implemented with it.
>
> This change is already reviewed by 2d folks.
> So I want to merge it ASAP.
>
> Do you have any objection?
>
>
> Thanks,
>
> Yasumasa
>
>
> [1] http://mail.openjdk.java.net/pipermail/2d-dev/2016-June/007090.html
> [2] http://mail.openjdk.java.net/pipermail/jdk9-dev/2016-June/004499.html
>
>
> On 2016/06/28 8:37, Kim Barrett wrote:
>>> On Jun 25, 2016, at 9:57 AM, Yasumasa Suenaga <yasuenag at gmail.com> 
>>> wrote:
>>>
>>> Hi all,
>>>
>>> This review request relates to [1].
>>>
>>> I've tried to build OpenJDK 9 at Fedora 24 x64.
>>> Fedora 24 has GCC 6.1.1, and OpenJDK 9 build was failed.
>>>
>>> I fixed build error and several issues (VM crash and internal error) 
>>> as below:
>>>
>>>  http://cr.openjdk.java.net/~ysuenaga/jdk9-for-gcc6/hotspot/
>>>
>>> Does someone work for it?
>>> If no one works for it, I will file it to JBS and will send review 
>>> request.
>>>
>>> For jdk repos, I've sent review request [2].
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>> [1] 
>>> http://mail.openjdk.java.net/pipermail/jdk9-dev/2016-June/004494.html
>>> [2] http://mail.openjdk.java.net/pipermail/2d-dev/2016-June/007081.html
>>
>> Having gone through these, I think all of them are arising due to
>> build system problems, where we seem to have lost the compiler
>> configuration to use explicit selection of the language standard and
>> some additional options.

Do tell more about what this means. Where would this previously have 
been seen ?
Different versions of Visual Studio / CLANG / GCC all emit different 
warnings
and it is not always monotonically increasing with compiler version, and I
can imagine someone might want to have different sets of flags in general
depending on compiler version in use, but I have not seen a pattern of this
being applied to the warnings on any of the platforms.

in the makefile here there is just one special case of this ..

474 # Suppress gcc warnings like "variable might be clobbered by 'longjmp'
  475 # or 'vfork'": this warning indicates that some variable is placed to
  476 # a register by optimized compiler and it's value might be lost on longjmp().
  477 # Recommended way to avoid such warning is to declare the variable as
  478 # volatile to prevent the optimization. However, this approach does not
  479 # work because we have to declare all variables as volatile in result.
  480 #ifndef CROSS_COMPILE_ARCH
  481 #  CC_43_OR_NEWER := \
  482 #      $(shell $(EXPR) $(CC_MAJORVER) \> 4 \| \
  483 #          \( $(CC_MAJORVER) = 4 \& $(CC_MINORVER) \>= 3 \) )
  484 #  ifeq ($(CC_43_OR_NEWER), 1)
  485 #    BUILD_LIBJAVAJPEG_CFLAGS_linux += -Wno-clobbered
  486 #  endif
  487 #endif


>>
>> For now I think we should fix the build system problems, and file
>> additional bugs or update existing ones as needed to fix the root
>> causes of the problems encountered. I think many of the proposed
>> changes do not address the root causes, and should not be made. See
>> my comments for the specific bugs.
>>
>> I'm not on the mailing list where the jdk RFR was submitted.  I took a
>> look at them though, and
>>
>> ------------------------------------------------------------------------------ 
>>
>> make/lib/Awt2dLibraries.gmk
>>  407 # Avoid warning for GCC 6
>>  408 ifeq ($(TOOLCHAIN_TYPE), gcc)
>>  409   LCMS_CFLAGS += -Wno-misleading-indentation
>>  410 endif
>>
>>  926     # Avoid warning for GCC 6
>>  927     ifeq ($(TOOLCHAIN_TYPE), gcc)
>>  928       BUILD_LIBSPLASHSCREEN_jdhuff.c_CFLAGS += 
>> -Wno-shift-negative-value
>>  929       BUILD_LIBSPLASHSCREEN_jdphuff.c_CFLAGS += 
>> -Wno-shift-negative-value
>>  930     endif
>>
>> The -Wmisleading-indentation and -Wshift-negative-value options are
>> new in gcc 6. gcc has for some time (starting with gcc 4.4) silently
>> ignored unrecognized -Wno-XXX options. But some folks (like SAP) are
>> still using older versions. So these will need to be conditionalized
>> on the gcc version.
>>
>> ------------------------------------------------------------------------------ 
>>
>> src/java.desktop/share/native/libfontmanager/layout/SunLayoutEngine.cpp
>>  154   if (min < 0) min = 0;
>>  155   if (max < min) max = min; /* defensive coding */
>>
>> [splitting the line]
>>
>> Seems like this would be suppressed by -Wno-misleading-indentation,
>> especially since the reported warning is for that.  Why change both
>> the code and the build configuration?

Was that something seen in the original fix ? It is not in the version I 
reviewed.
The current version of the fix does not update the makefile to add this
.. except for LCMS - which is a different library.


-phil.

>>
>> ------------------------------------------------------------------------------ 
>>
>>
>> The changes in AlphaMath.c and splashscreen_jpeg.c look ok.
>>