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

[Kyrill Tkachov]

Hi Christian,

On 08/12/15 12:53, Christian Bruel 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);
> }
>
> 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)
>

I agree, the vector types (re)initialisation is a tricky part.
I've seen similar issues in the aarch64 work for target attributes

  bool
  arm_vector_mode_supported_p (machine_mode mode)
  {
-  /* Neon also supports V2SImode, etc. listed in the clause below.  */
-  if (TARGET_NEON && (mode == V2SFmode || mode == V4SImode || mode == V8HImode
+  if (mode == V2SFmode || mode == V4SImode || mode == V8HImode
        || mode == V4HFmode || mode == V16QImode || mode == V4SFmode
-      || mode == V2DImode || mode == V8HFmode))
-    return true;
-
-  if ((TARGET_NEON || TARGET_IWMMXT)
-      && ((mode == V2SImode)
-	  || (mode == V4HImode)
-	  || (mode == V8QImode)))
+      || mode == V2DImode || mode == V8HFmode
+      || mode == V2SImode || mode == V4HImode || mode == V8QImode)
      return true;
  

So this allows vector modes unconditionally for all targets/fpu configurations?
I was tempted to do that in aarch64 when I was encountering similar issues.
In the end what worked for me was re-laying out the vector types in SET_CURRENT_FUNCTION
if necessary (https://gcc.gnu.org/ml/gcc-patches/2015-08/msg01084.html)

Kyrill