[aarch64-port-dev ] aarch64: jdk9: 8078245: fails to build from source

[Erik Joelsson]

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.joelsson at 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.