Thanks for the review!
> -----Original Message----- > From: David Holmes <[email protected]> > Sent: Montag, 13. August 2018 09:00 > To: Baesken, Matthias <[email protected]>; Phil Race > <[email protected]> > Cc: [email protected]; '[email protected]' <build- > [email protected]> > 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 <[email protected]> > > Sent: Freitag, 10. August 2018 20:12 > > To: Baesken, Matthias <[email protected]> > > Cc: '[email protected]' <[email protected]>; awt- > [email protected] > > 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' <[email protected]><mailto:[email protected]> > > Cc: '[email protected]<mailto:[email protected]>' > <[email protected]><mailto:[email protected]>; awt- > [email protected]<mailto:[email protected]> > > 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 > <[email protected]<mailto:[email protected]>> > > Sent: Donnerstag, 9. August 2018 04:09 > > To: Baesken, Matthias > <[email protected]<mailto:[email protected]>> > > Cc: '[email protected]<mailto:[email protected]>' > <[email protected]<mailto:[email protected]>>; awt- > [email protected]<mailto:[email protected]> > > 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 > <[email protected]><mailto:[email protected]> > > Sent: Dienstag, 7. August 2018 17:15 > > To: Baesken, Matthias > <[email protected]><mailto:[email protected]> > > Cc: Doerr, Martin > <[email protected]><mailto:[email protected]>; 2d- > [email protected]<mailto:[email protected]>; Simonis, Volker > <[email protected]><mailto:[email protected]>; Lindenmaier, > Goetz <[email protected]><mailto:[email protected]> > > 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' <[email protected]><mailto:[email protected]>; > Doerr, Martin <[email protected]><mailto:[email protected]> > > Cc: Simonis, Volker > <[email protected]><mailto:[email protected]>; Lindenmaier, > Goetz <[email protected]><mailto:[email protected]> > > 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 */ > > > > > > > >
