<AWT Dev> [RFR]: 8209115: adjust libsplashscreen linux ppc64le builds for easier libpng update - was : RE: RFR 8195615 : libsplashscreen linux ppc64le build error after libpng update

[David Holmes]

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.race at oracle.com>
> Sent: Freitag, 10. August 2018 20:12
> To: Baesken, Matthias <matthias.baesken at sap.com>
> Cc: 'build-dev at openjdk.java.net' <build-dev at openjdk.java.net>; awt-dev at 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.openjdk.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.race at oracle.com><mailto:philip.race at oracle.com>
> Cc: 'build-dev at openjdk.java.net<mailto:build-dev at openjdk.java.net>' <build-dev at openjdk.java.net><mailto:build-dev at openjdk.java.net>; awt-dev at openjdk.java.net<mailto:awt-dev at 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.race at oracle.com<mailto:philip.race at oracle.com>>
> Sent: Donnerstag, 9. August 2018 04:09
> To: Baesken, Matthias <matthias.baesken at sap.com<mailto:matthias.baesken at sap.com>>
> Cc: 'build-dev at openjdk.java.net<mailto:build-dev at openjdk.java.net>' <build-dev at openjdk.java.net<mailto:build-dev at openjdk.java.net>>; awt-dev at openjdk.java.net<mailto:awt-dev at 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.openjdk.java.net/%7Embaesken/webrevs/8209115.0/>
> 
> https://bugs.openjdk.java.net/browse/JDK-8209115
> 
> Thanks ,
>        Matthias
> 
> 
> 
> From: Philip Race <philip.race at oracle.com><mailto:philip.race at oracle.com>
> Sent: Dienstag, 7. August 2018 17:15
> To: Baesken, Matthias <matthias.baesken at sap.com><mailto:matthias.baesken at sap.com>
> Cc: Doerr, Martin <martin.doerr at sap.com><mailto:martin.doerr at sap.com>; 2d-dev at openjdk.java.net<mailto:2d-dev at openjdk.java.net>; Simonis, Volker <volker.simonis at sap.com><mailto:volker.simonis at sap.com>; Lindenmaier, Goetz <goetz.lindenmaier at sap.com><mailto:goetz.lindenmaier at 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.race at oracle.com><mailto:philip.race at oracle.com>; Doerr, Martin <martin.doerr at sap.com><mailto:martin.doerr at sap.com>
> Cc: Simonis, Volker <volker.simonis at sap.com><mailto:volker.simonis at sap.com>; Lindenmaier, Goetz <goetz.lindenmaier at sap.com><mailto:goetz.lindenmaier at 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 */
> 
> 
> 
>