[PATCH] D67159: [clang] New __attribute__((__clang_builtin)).

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D67159#1659103 <https://reviews.llvm.org/D67159#1659103>, @simon_tatham wrote:

> Come to think of it, it would also not be too hard to constrain it to //only// be usable for a particular subset of builtins, and perhaps even only with a particular set of alias names for them. (I could easily derive all that information from the same Tablegen that `arm_mve.h` itself is made from.)


I think this might be a good idea to explore. In that case, I would recommend naming the attribute `__clang_arm_mve_builtin` to make it obvious that this attribute has a very specific use in mind.



================
Comment at: clang/include/clang/Basic/Attr.td:596
 }
+def ClangBuiltinOverride : Attr {
+  let Spellings = [GCC<"__clang_builtin">];
----------------
simon_tatham wrote:
> aaron.ballman wrote:
> > Do you expect this attribute to be inherited on redeclarations? I suspect this should be an `InheritableAttr`.
> > 
> > Also, add a newline above it for visual separation, please.
> > 
> > Finally, should this be a target-specific attribute so that it's only available for your target, or do you expect this attribute to be used on all target architectures?
> For my use case, I have no opinion about redeclarations: I expect to declare each affected function exactly once. If you think `InheritableAttr` is a more sensible default choice, I'm fine with that.
> 
> Target-specific: I don't have a use case outside the ARM target, so I'd be happy to lock it down that way if you want.
I think it should be an `InheritableAttr` that is target-specific. We can always expand the targets later if we think the attribute is generally useful.


================
Comment at: clang/include/clang/Basic/Attr.td:600
+  let Subjects = SubjectList<[Function], ErrorDiag>;
+  let Documentation = [Undocumented];
+}
----------------
simon_tatham wrote:
> aaron.ballman wrote:
> > No new undocumented attributes, please.
> OK. I'd intended to leave it undocumented in order to discourage people from using it in any context //other// than a system header file. But fair enough – perhaps it should be documented even so.
Documenting it still helps developers even if the documentation effectively states that something is for internal use only.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67159/new/

https://reviews.llvm.org/D67159