Hi Nevill,

Just realized this was sent to hotspot-dev (attempting bcc) but is not a hotspot issue. With your new approach this is a build issue so cc'ing build-dev.

The new approach seems better to me but build folk need to confirm the placement.

Thanks,
David

On 29/04/2015 8:52 PM, Edward Nevill wrote:
On Fri, 2015-04-24 at 17:11 +1000, David Holmes wrote:
Hi Ed,

On 21/04/2015 7:16 PM, Edward Nevill wrote:
Hi,

The current jdk9 tip fails to build from source on aarch64 with the following 
error message

/home/ed/build/1504/dev/build/linux-aarch64-normal-server-release/support/native/java.desktop/libsplashscreen/pngrutil.o:
 In function `png_init_filter_functions':
/home/ed/build/1504/dev/jdk/src/java.desktop/share/native/libsplashscreen/libpng/pngrutil.c:3947:
 undefined reference to `png_init_filter_functions_neon'
collect2: error: ld returned 1 exit status

The following webrev gets it building again.

http://cr.openjdk.java.net/~enevill/8078245/webrev.00/

Shouldn't the guard be Aarch64 specific rather than just __arm__ ? I'm
also wondering how we would get __ARM_NEON defined but not __arm__?

On arm 32 bit gcc defines the symbol __ARM_NEON if the flag -mfpu=neon is 
specified. Since this is not specified as part of the OpenJDK build the symbol 
is not defined and the build succeeds.

On aarch64 the symbol __ARM_NEON is always defined (the theory being that 
aarch64 always supports Neon). Personally I think this is borken as it causes 
builds to fail (and not just OpenJDK, several other projects have had the same 
build failure - try googling the above error message). But we are stuck with 
gcc as it is.

However, I don't like the above fix because it not only modifies the jdk, it 
modifies an external component which is pulled into jdk, which means every time 
a new revision of linpng is pulled in, the patch will have to be applied again.

A better approach I think is to define the symbol PNG_ARM_NEON_OPT=0 in the 
build (only if aarch64).

The failing code in pngpriv.h reads

#ifndef PNG_ARM_NEON_OPT

...

#  if (defined(__ARM_NEON__) || defined(__ARM_NEON)) && \
    defined(PNG_ALIGNED_MEMORY_SUPPORTED)
#     define PNG_ARM_NEON_OPT 2
#  else
#     define PNG_ARM_NEON_OPT 0
#  endif
#endif

So, if we just predefine PNG_ARM_NEON=0 in the build this will have the same 
effect as adding defined(__arm__) or !defined(__aarch64__) above.

The following patch webrev does this:-

http://cr.openjdk.java.net/~enevill/8078245/webrev.01/

If you are happy with this and if I could have another reviewer I will push 
this.

All the best,
Ed.


Reply via email to