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

[James Molloy]

Hi Alp,

Thanks for the comments. New patch attached, is it any better? I'm not sure
exactly what style we normally use for the diagnostics.

Cheers,

James

> -----Original Message-----
> From: Alp Toker [mailto:alp at nuanti.com]
> Sent: 19 June 2014 10:12
> To: James Molloy; Clang Commits
> Subject: Re: [PATCH] Warn on use of vector initializers in ARM BE mode
> 
> 
> 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
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-The-ability-to-use-vector-initializer-lists-is-a-GNU.patch
Type: application/octet-stream
Size: 6210 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140619/4b60df09/attachment.obj>