[PATCH] D100611: [RISCV] Add new attribute __clang_riscv_builtin_alias for intrinsics.

Tue Apr 20 05:35:00 PDT 2021

aaron.ballman added inline comments.

================
Comment at: clang/include/clang/Basic/Attr.td:1771
+def RISCVBuiltinAlias : InheritableAttr, TargetSpecificAttr<TargetRISCV> {
+  let Spellings = [Clang<"__clang_riscv_builtin_alias">];
+  let Subjects = SubjectList<[Function], ErrorDiag>;
----------------
Why is this being given a reserved identifier name? That's pretty atypical for attributes as attributes can optionally be prefixed and suffixed with `__` already.

================
Comment at: clang/include/clang/Basic/AttrDocs.td:2147-2150
+This attribute can only be used to set up the aliases for certain RISC-V
+C intrinsic functions; it is intended for use only inside ``riscv_*.h``
+and is not a general mechanism for declaring arbitrary aliases for
+clang builtin functions.
----------------
Why a specific attribute for only one architecture rather than a more general solution that can be used for multiple architectures? We already have `__clang_arm_builtin_alias` (with the incorrect name, so I guess I see where you got the __ from) and now we're extending it. I am not certain if there's a reason why we should use a different attribute for each target though.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100611