Re: [PATCH, ARM] PR68674 Fix LTO support for neon builtins and error catching

[Ramana Radhakrishnan <ramana dot gcc at googlemail dot com>]

On Tue, Dec 8, 2015 at 12:53 PM, Christian Bruel <christian.bruel@st.com> wrote:
> Hi,
>
> The order of the NEON builtins construction has led to complications since
> the attribute target support. This was not a problem when driven from the
> command line, but was causing various issues when the builtins was mixed
> between fpu configurations or when used with LTO.
>
> Firstly the builtin functions was not initialized before the parsing of
> functions, leading to wrong type initializations.
>
> Then error catching code when a builtin was used without the proper fpu
> flags was incomprehensible for the user, for instance
>
> #include "arm_neon.h"
>
> int8x8_t a, b;
> int16x8_t e;
>
> void
> main()
> {
>   e = (int16x8_t)__builtin_neon_vaddlsv8qi (a, b);
> }

I'm not sure what problem you are trying to solve here - The user
should never be using __builtin_neon_vaddlsv8qi (a, b) here. What
happens with vaddl_s16 intrinsic ?

They really have to only use the vaddl_s8 intrinsic.


Ramana

>
> compiled with default options (without -mfpu=neon -mfloat-abi=hard) gave
> pages of
>
> /arm-none-eabi/6.0.0/include/arm_neon.h:39:9: error: unknown type name
> '__simd64_int8_t'
>  typedef __simd64_int8_t int8x8_t;
> ...
> ...
> arm_neon.h:4724:3: error: can't convert a vector of type 'poly64x2_t {aka
> __vector(4) int}' to type 'int' which has different size
>    return (poly64x2_t)__builtin_neon_vsli_nv2di ((int64x2_t) __a,
> (int64x2_t) __b, __c);
>    ^~~~~~
> ...
> ... and one for each arm_neon.h lines..
>
> by postponing the check into arm_expand_builtin, we now emit something more
> useful:
>
> testo.c: In function 'main':
> testo.c:9:7: error: '__builtin_neon_vaddlsv8qi' neon builtin is not
> supported in this configuration.
>    e = (int16x8_t)__builtin_neon_vaddlsv8qi (a, b);
>        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> One small side effect to note: The total memory allocated is 370k bigger
> when neon is not used, so this support will have a follow-up to make their
> initialization lazy. But I'd like first to stabilize the stuff for stage3
> (or get it pre-approved if the memory is an issue)
>
> tested without new failures with {,-mfpu=vfp,-mfpu=neon}{,-march=armv7-a\}
> (a few tests that was fail are now unsupported)
>
> OK for trunk ?
>
>
>
>
>
>
>
>