Hello Ed,
I'm happy with this.
/Erik
On 2015-05-05 11:23, Edward Nevill wrote:
Hi,
You are right, there is a better place for this,
in make/lib/Awt2dLibraries.gmk b/make/lib/Awt2dLibraries.gmk
is the line
LIBSPLASHSCREEN_CFLAGS += -DSPLASHSCREEN -DPNG_NO_MMX_CODE \
so, someone has already done this here for x86, so I just need to add the
equivalent for aarch64
LIBSPLASHSCREEN_CFLAGS += -DSPLASHSCREEN -DPNG_NO_MMX_CODE
-DPNG_ARM_NEON_OPT=0 \
Note: I have not conditionalised this on OPENJDK_TARGET_CPU_ARCH as the
corresponding x86 code is not conditionalised and I really don't think the
symbol PNG_ARM_NEON_OPT will have any effect on any other architectures.
Erik, Magnus & David, I have listed you as reviewers in the following webrev.
If you are happy with this please let me know and I will push.
http://cr.openjdk.java.net/~enevill/8078245/webrev.02/
All the best,
Ed.
On Mon, 2015-05-04 at 17:03 +0200, Erik Joelsson wrote:
Right, that does make sense. This isn't a toolchain global flag like
some others we've had to deal with lately, but rather seems pretty
specific to the png code, or am I missing something? If that's the case,
then it's better added for the specific library.
/Erik
On 2015-05-04 16:43, Magnus Ihse Bursie wrote:
Why not just for the library that needs it?
/Magnus
4 maj 2015 kl. 11:05 skrev Erik Joelsson <erik.joels...@oracle.com>:
Hello,
I think this looks good enough. The flags handling is still a big mess so even
if there were a better place for this, I couldn't say where.
/Erik
On 2015-04-30 07:26, David Holmes wrote:
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.
