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

Tue Apr 20 23:32:46 PDT 2021

HsiangKai added inline comments.

================
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.
----------------
HsiangKai wrote:
> aaron.ballman wrote:
> > 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.
> I agree with you. We should not define the attribute as target specific. This patch is a quick solution to address the issue https://bugs.llvm.org/show_bug.cgi?id=49962.
> 
> I will refactor this patch and try to merge it with `__clang_arm_builtin_alias` in some way.
@aaron.ballman Could we land this patch first to reduce the test time?
I agree with you that it is better to have one unified attribute for the purpose. However, if we change the attribute defined by ARM, we need to negotiate with them and there may be already some use cases in their users. I am not sure the impact of the modification.

I could modify my patch to make the attribute target independent and leave ARM's attribute as before. What do you think?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100611