Thanks for the review!
> -----Original Message----- > From: David Holmes <david.hol...@oracle.com> > Sent: Montag, 13. August 2018 09:00 > To: Baesken, Matthias <matthias.baes...@sap.com>; Phil Race > <philip.r...@oracle.com> > Cc: awt-...@openjdk.java.net; 'build-dev@openjdk.java.net' <build- > d...@openjdk.java.net> > Subject: Re: [RFR]: 8209115: adjust libsplashscreen linux ppc64le builds for > easier libpng update - was : RE: RFR 8195615 : libsplashscreen linux ppc64le > build error after libpng update > > Hi Matthias, > > On 13/08/2018 4:41 PM, Baesken, Matthias wrote: > > Thank‘s ! > > Can I have a second review please ? > > As the build team seem to be on vacation right now I can add that second > Review. :) > > Cheers, > David > > > Best regards, Matthias > > > > > > From: Phil Race <philip.r...@oracle.com> > > Sent: Freitag, 10. August 2018 20:12 > > To: Baesken, Matthias <matthias.baes...@sap.com> > > Cc: 'build-dev@openjdk.java.net' <build-dev@openjdk.java.net>; awt- > d...@openjdk.java.net > > Subject: Re: [RFR]: 8209115: adjust libsplashscreen linux ppc64le builds for > easier libpng update - was : RE: RFR 8195615 : libsplashscreen linux ppc64le > build error after libpng update > > > > +1 from me. > > > > -phil. > > On 08/10/2018 08:18 AM, Baesken, Matthias wrote: > > Hello , here is a new webrev : > > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8209115.1/<http://cr.open > jdk.java.net/%7Embaesken/webrevs/8209115.1/> > > > > I set PNG_POWERPC_VSX_OPT now only on the linux ppc platforms . > > > > Best regards, Matthias > > > > > > From: Baesken, Matthias > > Sent: Donnerstag, 9. August 2018 08:48 > > To: 'Philip Race' <philip.r...@oracle.com><mailto:philip.r...@oracle.com> > > Cc: 'build-dev@openjdk.java.net<mailto:build-dev@openjdk.java.net>' > <build-dev@openjdk.java.net><mailto:build-dev@openjdk.java.net>; awt- > d...@openjdk.java.net<mailto:awt-...@openjdk.java.net> > > Subject: RE: [RFR]: 8209115: adjust libsplashscreen linux ppc64le builds for > easier libpng update - was : RE: RFR 8195615 : libsplashscreen linux ppc64le > build error after libpng update > > > > > > Ø Seems like the ARM one has been harmless for an x64 build > > > > * so we could for now assume the PPC one is too > > > > Hi Phil , > > I will guard the define for the platform where we want to use > > it - I > think this is better for the future than just setting it on all platforms . > > > > And btw. it has to be > > > > PNG_POWERPC_VSX_OPT not PNG_POWERPC_VSX > > > > ☹ - my mistake . > > > > For some reason the current setting still builds on our ppc64 (le) test > machine in the current codebase , strange ! > > > > Best regards, Matthias > > > > > > From: Philip Race > <philip.r...@oracle.com<mailto:philip.r...@oracle.com>> > > Sent: Donnerstag, 9. August 2018 04:09 > > To: Baesken, Matthias > <matthias.baes...@sap.com<mailto:matthias.baes...@sap.com>> > > Cc: 'build-dev@openjdk.java.net<mailto:build-dev@openjdk.java.net>' > <build-dev@openjdk.java.net<mailto:build-dev@openjdk.java.net>>; awt- > d...@openjdk.java.net<mailto:awt-...@openjdk.java.net> > > Subject: Re: [RFR]: 8209115: adjust libsplashscreen linux ppc64le builds for > easier libpng update - was : RE: RFR 8195615 : libsplashscreen linux ppc64le > build error after libpng update > > > > > > The only comment I have is about this > > > > - LIBSPLASHSCREEN_CFLAGS += -DSPLASHSCREEN - > DPNG_NO_MMX_CODE -DPNG_ARM_NEON_OPT=0 > > > > + LIBSPLASHSCREEN_CFLAGS += -DSPLASHSCREEN - > DPNG_NO_MMX_CODE -DPNG_ARM_NEON_OPT=0 - > DPNG_POWERPC_VSX=0 > > > > > > > > I suppose these platform defines are harmless and un-referenced unless > building for > > > > the specified platform. Seems like the ARM one has been harmless for an > x64 build > > > > so we could for now assume the PPC one is too. > > > > Or you could use jdk-submit to prove this :-) > > > > > > > > -phil. > > > > On 8/8/18, 12:59 AM, Baesken, Matthias wrote: > > Hello , > > please review this small adjustment for the build of libsplashscreen , > especially the libpng parts . > > Back then, we had to fix the build of the libpng parts in > > libsplashscreen > on linux ppc64le with “8195615 : libsplashscreen linux ppc64le build error > after libpng update” . > > > > However this introduced a small adjustment to pngpriv.h that needs to be > kept every time libpng is updated (happened recently when Phil updated > the lib). > > So I better want to remove the adjustment to pngpriv from 8195615 , > and instead change the build settings to fix the compilation on linuxppc64 > le . > > > > Webrev/bug : > > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8209115.0/<http://cr.open > jdk.java.net/%7Embaesken/webrevs/8209115.0/> > > > > https://bugs.openjdk.java.net/browse/JDK-8209115 > > > > Thanks , > > Matthias > > > > > > > > From: Philip Race > <philip.r...@oracle.com><mailto:philip.r...@oracle.com> > > Sent: Dienstag, 7. August 2018 17:15 > > To: Baesken, Matthias > <matthias.baes...@sap.com><mailto:matthias.baes...@sap.com> > > Cc: Doerr, Martin > <martin.do...@sap.com><mailto:martin.do...@sap.com>; 2d- > d...@openjdk.java.net<mailto:2d-...@openjdk.java.net>; Simonis, Volker > <volker.simo...@sap.com><mailto:volker.simo...@sap.com>; Lindenmaier, > Goetz <goetz.lindenma...@sap.com><mailto:goetz.lindenma...@sap.com> > > Subject: Re: libsplashscreen compilation on ppc64 ( le) - was : RE: RFR > 8195615 : libsplashscreen linux ppc64le build error after libpng update > > > > Works for me. Include build-dev on the review. > > And splashscreen is considered an AWT feature so it should be awt-dev not > 2d-dev > > although you may want to reference back to this earlier exchange. > > > > -phil. > > > > On 8/7/18, 8:04 AM, Baesken, Matthias wrote: > > Hello, should I prepare a change setting the -DPNG_POWERPC_VSX=0 > flag in the makefile (see below) ? > > Might make future libpng updates more simple . > > > > Best regards, Matthias > > > > From: Baesken, Matthias > > Sent: Donnerstag, 2. August 2018 17:28 > > To: 'Phil Race' <philip.r...@oracle.com><mailto:philip.r...@oracle.com>; > Doerr, Martin <martin.do...@sap.com><mailto:martin.do...@sap.com> > > Cc: Simonis, Volker > <volker.simo...@sap.com><mailto:volker.simo...@sap.com>; Lindenmaier, > Goetz <goetz.lindenma...@sap.com><mailto:goetz.lindenma...@sap.com> > > Subject: RE: RFR 8195615 : libsplashscreen linux ppc64le build error after > libpng update - was : RE: jdk-hs ppc64le build error, probably related to > libpng > update > > > > Hi Phil, I added -DPNG_POWERPC_VSX=0 to Awt2dLibraries.gmk for the > sphlashscreen library build, and removed (uncommented) the workaround > in pngpriv.h . > > Build on the head of jdk11 was fine on my linux ppc64le test > > machine . > > > > Best regards, Matthias > > > > Diff: > > > > > > /open_jdk/jdk11> hg diff > > diff -r 26cca23c165a make/lib/Awt2dLibraries.gmk > > --- a/make/lib/Awt2dLibraries.gmk Thu Aug 02 09:49:04 2018 +0200 > > +++ b/make/lib/Awt2dLibraries.gmk Thu Aug 02 16:50:09 2018 +0200 > > @@ -794,7 +794,8 @@ > > LIBSPLASHSCREEN_EXCLUDE_SRC_PATTERNS := unix > > endif > > > > - LIBSPLASHSCREEN_CFLAGS += -DSPLASHSCREEN - > DPNG_NO_MMX_CODE -DPNG_ARM_NEON_OPT=0 > > + # disable ppc64 opts > > + LIBSPLASHSCREEN_CFLAGS += -DSPLASHSCREEN - > DPNG_NO_MMX_CODE -DPNG_ARM_NEON_OPT=0 - > DPNG_POWERPC_VSX=0 > > > > ifeq ($(OPENJDK_TARGET_OS), macosx) > > LIBSPLASHSCREEN_CFLAGS += -DWITH_MACOSX > > diff -r 26cca23c165a > src/java.desktop/share/native/libsplashscreen/libpng/pngpriv.h > > --- a/src/java.desktop/share/native/libsplashscreen/libpng/pngpriv.h Thu > Aug 02 09:49:04 2018 +0200 > > +++ b/src/java.desktop/share/native/libsplashscreen/libpng/pngpriv.h > Thu Aug 02 16:50:09 2018 +0200 > > @@ -290,12 +290,12 @@ > > # endif > > #endif /* PNG_MIPS_MSA_OPT > 0 */ > > > > -#ifdef PNG_POWERPC_VSX_API_SUPPORTED > > +/* #ifdef PNG_POWERPC_VSX_API_SUPPORTED */ > > #if PNG_POWERPC_VSX_OPT > 0 > > # define PNG_FILTER_OPTIMIZATIONS png_init_filter_functions_vsx > > # define PNG_POWERPC_VSX_IMPLEMENTATION 1 > > #endif > > -#endif > > +/* #endif */ > > > > > > > >
