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 <yasue...@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.
