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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
