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

Thu Jun 30 00:44:29 UTC 2016

Hi,

Not just yet. Whilst I am OK with it ...

1) We like 2 (two) reviewers to approve.
2) I would like Kim to reply to the questions so I understand
his concerns first.

-phil.

On 6/29/16, 4:30 PM, Yasumasa Suenaga wrote:
> Hi Kim, Phil,
>
> Can I push this patch?
> It has been reviewed.
>
>   http://cr.openjdk.java.net/~ysuenaga/JDK-8160294/webrev.01/
>
>
> Thanks,
>
> Yasumasa
>
>
> On 2016/06/29 2:38, Phil Race wrote:
>> 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.
>>>>
>>