[PATCH] Warn on use of vector initializers in ARM BE mode

[Alp Toker]

On 19/06/2014 14:51, James Molloy wrote:
> Hi Tim, Alp,
>
> Tim, you're right. However Alp I think has the solution - if we only fire when a NEON vector type is used, it restricts to only types that are defined in arm_neon.h.
>
> Alp, I implemented both of your suggestions, and a new patch is attached.
>
> Thanks for catching both of these!

>  #include "clang/AST/ExprCXX.h"
>  #include "clang/AST/ExprObjC.h"
>  #include "clang/AST/TypeLoc.h"
> +#include "clang/Basic/TargetInfo.h"
>  #include "clang/Sema/Designator.h"
>  #include "clang/Sema/Lookup.h"
>  #include "clang/Sema/SemaInternal.h"
> @@ -1204,6 +1205,46 @@ void InitListChecker::CheckVectorType(const 
> InitializedEntity &Entity,
>        CheckSubElementType(ElementEntity, IList, elementType, Index,
>                            StructuredList, StructuredIndex);
>      }
> +
> +    llvm::Triple Triple = SemaRef.Context.getTargetInfo().getTriple();

You can drop the include and the triple now as they're unused.

> --- /dev/null
> +++ b/test/Sema/big-endian-neon-initializers.c
> @@ -0,0 +1,9 @@
> +// RUN: %clang_cc1 %s -triple arm64_be -target-feature +neon -verify 
> -fsyntax-only -ffreestanding
> +// RUN: %clang_cc1 %s -triple armebv7 -target-cpu cortex-a8 -verify 
> -fsyntax-only -ffreestanding
> +
> +#include <arm_neon.h>
> +
> +int32x4_t x = {1, 2, 3, 4}; // expected-warning{{vector initializers 
> are a GNU extension and are not compatible with NEON intrinsics}} 
> expected-note{{consider using vld1q_s32() to initialize a vector from 
> memory, or vcombine_s32(vcreate_s32(), vcreate_s32()) to initialize 
> from integer constants}}
> +int16x4_t y = {1, 2, 3, 4}; // expected-warning{{vector initializers 
> are a GNU extension and are not compatible with NEON intrinsics}} 
> expected-note{{consider using vld1_s16() to initialize a vector from 
> memory, or vcreate_s16() to initialize from an integer constant}}
> +int64x2_t z = {1, 2}; // expected-warning{{vector initializers are a 
> GNU extension and are not compatible with NEON intrinsics}} 
> expected-note{{consider using vld1q_s64() to initialize a vector from 
> memory, or vcombine_s64(vcreate_s64(), vcreate_s64()) to initialize 
> from integer constants}}
> +float32x2_t b = {1, 2}; // expected-warning{{vector initializers are 
> a GNU extension and are not compatible with NEON intrinsics}} 
> expected-note{{consider using vld1_f32() to initialize a vector from 
> memory, or vcreate_f32() to initialize from an integer constant}}

Could you add test cases for non-Neon vector initializations like the 
ones Tim mentioned that _shouldn't_ trigger the warning?

As for the warning option name, I agree with Tim's comment that this 
should have its own. How about dropping the 'GNU' mentions entirely, and 
keeping the message straightforward like "vector initialization of NEON 
types is not portable"? Especially since our check isn't related to the 
actual use of intrinsics.

For a warning flag name, dig through similar warnings in the td files 
for inspiration, perhaps something like Wnonportable-vector-initialization.

Alp.




>
> James
>
>> -----Original Message-----
>> From: Tim Northover [mailto:t.p.northover at gmail.com]
>> Sent: 19 June 2014 12:39
>> To: James Molloy
>> Cc: Alp Toker; Clang Commits
>> Subject: Re: [PATCH] Warn on use of vector initializers in ARM BE mode
>>
>> Hi James,
>>
>> On 19 June 2014 11:13, James Molloy <james.molloy at arm.com> wrote:
>>> Thanks for the comments. New patch attached, is it any better? I'm not
>> sure
>>> exactly what style we normally use for the diagnostics.
>> Doesn't this warning trigger even if arm_neon.h is nowhere to be
>> found? That seems a bit broad, the GNU vector extensions are useful in
>> their own right, and I'm not sure we should be effectively deprecating
>> them on ARM.
>>
>> Unfortunately, I've got no ideas on how to only give a message when
>> it's relevant. I don't really do much on clang either.
>>
>> Other than that, you need a Triple::aarch64 check too.
>>
>> Cheers.
>>
>> Tim.

-- 
http://www.nuanti.com
the browser experts