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

[Alp Toker]

On 19/06/2014 11:53, James Molloy wrote:
> Hi Alp,
>
> I certainly thought about this, however it's not exactly clear how to
> generate good fixits. I can generate fixits that "kind of work", but my
> understanding was that the rule of thumb is that we shouldn't generate a
> fixit unless we're pretty sure it's correct.
>
> The main problem is that we must create a fixit on all the usage sites. The
> fix would be:
>
>    1. Convert the vector declaration into an array declaration.
>    2. Add vld* calls on each use of the declaration.
>
> Which is fairly difficult in itself (updating all uses of a decl is more
> difficult than just adding a warning on the decl itself), but also to
> maintain the semantics of the program we'd need to add vst* calls (store the
> result back) whenever the declaration may be modified either by arithmetic
> or an intrinsic.
>
> Because of this complexity, I felt it wasn't really worth the investment to
> generate proper fixits.

Fair enough if you don't see a reasonably straightforward syntactic FixIt.

The patch as-is does need some tweaking, some quick observations:

If reporting a portability issue, it makes sense to report the issue on 
either endianness, not just the big-endian targets where it's known to 
cause trouble.

Lowercase first letters for diag messages.

"are part of the GNU vector extensions ..." wording feels out of place 
compared to the usual terminology.

Alp.



>
> Cheers,
>
> James
>
>> -----Original Message-----
>> From: Alp Toker [mailto:alp at nuanti.com]
>> Sent: 19 June 2014 09:44
>> To: James Molloy; Clang Commits
>> Subject: Re: [PATCH] Warn on use of vector initializers in ARM BE mode
>>
>>
>> On 19/06/2014 10:58, James Molloy wrote:
>>> Hi,
>>>
>>> The ability to use vector initializer lists is a GNU vector extension
>>> and is unrelated to the NEON intrinsics in arm_neon.h. On little
>>> endian machines it works fine, however on big endian machines it
>>> exhibits surprising behaviour:
>>>
>>> uint32x2_t x = {42, 64};
>>>
>>> return vget_lane_u32(x, 0); // Will return 64.
>>>
>>> Because of this, explicitly call out that it is unsupported on big
>>> endian machines.
>>>
>>> This patch will emit the following warning in big-endian mode:
>>>
>>> test.c:3:15: warning: Vector initializers are part of the GNU vector
>>> extensions and are not compatible with NEON intrinsics on big endian
>>> ARM targets [-Wgnu]
>>>
>>> int32x4_t x = {0, 1, 2, 3};
>>>
>>> ^
>>>
>>> test.c:3:15: note: Consider using vld1q_s32() to initialize a vector
>>> from memory, or vcombine_s32(vcreate_s32(), vcreate_s32()) to
>>> initialize from integer constants
>>>
>>> 1 warning generated.
>>>
>>> It's fairly simple, is it OK to commit?
>>>
>> Hi James,
>>
>> Would it be a big step to generate FixIts instead of describing the
>> possible fixes in words?
>>
>> I think as a rule of thumb, if it's worth describing code changes in
>> diagnostics, it's worth attempting a FixIt.
>>
>> Alp.
>>
>>
>>> Cheers,
>>>
>>> James
>>>
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>> --
>> http://www.nuanti.com
>> the browser experts
>>
>
>
>

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