[Clang] Add clang attribute `clang_builtin_alias`.

[HsiangKai]

Summary

In some cases, we want to provide the alias name for the clang builtins.
For example, the arguments must be constant integers for some RISC-V builtins.
If we use wrapper functions, we could not constrain the arguments be constant
integer. This attribute is used to achieve the purpose.

Besides this, use clang_builtin_alias is more efficient than using
wrapper functions. We use this attribute to deal with test time issue
reported in https://bugs.llvm.org/show_bug.cgi?id=49962.

In our downstream testing, it could decrease the testing time from 6.3
seconds to 3.7 seconds for vloxei.c test.

[kito-cheng]

clang/test/CodeGen/RISCV/riscv-attr-builtin-alias.c

10

I guess this is not needed anymore? or at least could be reduced?

[kito-cheng]

Could you also check the compiler diagnostic messages? it will report __builtin_rvv_vadd_vv_i8m1 or vadd_generic if argument type mis-match, which one you expected? I assume without __clang_riscv_builtin_alias clang will report vadd_generic?

[frasercrmck]

Is this idea that this will later be "automatically" applied to builtins in riscv_vector.h?

[HsiangKai]

In D100611#2694004, @frasercrmck wrote:

Is this idea that this will later be "automatically" applied to builtins in riscv_vector.h?

I will prepare another patch to update the header generator.

[HsiangKai]

Add a negative test case.

[khchen]

Look good to me.
BTW, we also need to update the document later.

[HsiangKai]

Update document.

[HsiangKai]

In D100611#2697055, @khchen wrote:

Look good to me.
BTW, we also need to update the document later.

AttributeReference.html will be updated according to the content of AttrDocs.td.

[khchen]

Is it okay to you? @kito-cheng @frasercrmck
This patch could fix https://bugs.llvm.org/show_bug.cgi?id=49962, I hope this could be merged soon.

[frasercrmck]

clang/include/clang/Basic/AttrDocs.td

2141

Is vget real? I can't see it in riscv_vector.h

[HsiangKai]

Refine the document.

[HsiangKai]

clang/include/clang/Basic/AttrDocs.td

2141

Yes, it is for Zvlsseg. I have not upstreamed it. I use vadd instead in the description.

[frasercrmck]

LGTM. Anything else would be wondering if it can be merged/genericised with Arm somehow but that's not a blocker.

clang/include/clang/Basic/AttrDocs.td

2141

Ah, great. I think vadd's a good example.

[aaron.ballman]

clang/include/clang/Basic/Attr.td

1780

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.

clang/include/clang/Basic/AttrDocs.td

2147–2150

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.

[HsiangKai]

clang/include/clang/Basic/AttrDocs.td

2147–2150

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.

[HsiangKai]

clang/include/clang/Basic/AttrDocs.td

2147–2150

@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?

[HsiangKai]

@aaron.ballman, could we discuss the generalization in https://reviews.llvm.org/D100930?

[aaron.ballman]

clang/include/clang/Basic/AttrDocs.td

2147–2150

As best I can tell, at least from the attribute that we surface for the users, there shouldn't be much difference between the RISC-V and the ARM attributes. They're both accepted on the same subjects, have the same attribute arguments, and have the same semantic effect, etc. It seems like the only difference is with semantic checking of whether the builtin is valid for the architecture. Based on the assumption that these are effectively doing the same thing, I'd rather we land the generic attribute so that we can deprecate __clang_arm_builtin_alias in favor of the general attribute. If we add __clang_riscv_builtin_alias before adding the generic attribute, it's one more thing for us to deprecate and then remove.

Note, I'd be totally fine if the generic attribute only handles ARM and RISC-V builtins initially (and error on the other architectures). We can add support for other architectures as the need arises.

Do you think that approach makes sense?

[HsiangKai]

clang/include/clang/Basic/AttrDocs.td

2147–2150

It makes sense. I will integrate D100930 into this one.

[HsiangKai]

Add clang attribute clang_builtin_alias.

[HsiangKai]

Update test case.

[HsiangKai]

@aaron.ballman, could you help me to review it again? Thanks.

[aaron.ballman]

clang/include/clang/Basic/Attr.td

646

Hrm, this is interesting -- this will allow the user to write __attribute__((clang_builtin_alias(...))) which is nice, but it also allows [[clang::clang_builtin_alias(...)]] which duplicates the clang in that spelling and feels a bit awkward.

While we could steal __attribute__((builtin_alias)) as it seems like no one is using it, that feels a bit heavy-handed as builtins may not be portable across compilers.

Based on that, I sort of think we should go with:

let Spellings = [CXX11<"clang", "builtin_alias">, C2x<"clang", "builtin_alias">, GNU<"clang_builtin_alias">];

What do you think?

clang/include/clang/Basic/AttrDocs.td

4702

Can you also update ArmBuiltinAliasDocs to point users to the more general attribute?

clang/include/clang/Basic/DiagnosticSemaKinds.td

4030

clang/lib/Sema/SemaDeclAttr.cpp

5181

[HsiangKai]

clang/include/clang/Basic/Attr.td

646

It makes sense to me.

[HsiangKai]

Address comments.

[aaron.ballman]

LGTM aside from a small nit with the documentation wording.

clang/include/clang/Basic/AttrDocs.td

5716

I'd like to avoid saying "deprecated" because then people may expect to see warnings about use of the ARM attribute. I don't know if the ARM folks are ready for it to be officially deprecated as opposed to just discouraged.

[rnk]

clang/lib/Sema/SemaDeclAttr.cpp

5155

@vsk @HsiangKai I refactored this in rGd2f4b7d77828 to avoid the switch because it really messed up code coverage. It lead to a single .lcovfun section of a few gigabytes. That might be worth a look, and I recommend against this code pattern in the future.

clang/include/clang/Basic/Attr.td

 
 def Alias : Attr {
   let Spellings = [GCC<"alias">];
   let Args = [StringArgument<"Aliasee">];
   let Subjects = SubjectList<[Function, GlobalVar], ErrorDiag>;
   let Documentation = [Undocumented];
 }
 
+def BuiltinAlias : Attr {
+  let Spellings = [CXX11<"clang", "builtin_alias">,
+                   C2x<"clang", "builtin_alias">,
+                   GNU<"clang_builtin_alias">];
+  let Args = [IdentifierArgument<"BuiltinName">];
+  let Subjects = SubjectList<[Function], ErrorDiag>;
+  let Documentation = [BuiltinAliasDocs];
+}
+
 def ArmBuiltinAlias : InheritableAttr, TargetSpecificAttr<TargetAnyArm> {
   let Spellings = [Clang<"__clang_arm_builtin_alias">];
   let Args = [IdentifierArgument<"BuiltinName">];
   let Subjects = SubjectList<[Function], ErrorDiag>;
   let Documentation = [ArmBuiltinAliasDocs];
 }
 
 def Aligned : InheritableAttr {
let Args = [EnumArgument<"Interrupt", "InterruptType",
                            ["user", "supervisor", "machine"],
                            ["user", "supervisor", "machine"],
                            1>];
   let ParseKind = "Interrupt";
   let Documentation = [RISCVInterruptDocs];
 }
 
 // This is not a TargetSpecificAttr so that is silently accepted and
 // ignored on other targets as encouraged by the OpenCL spec.
 //
 // See OpenCL 1.2 6.11.5: "It is our intention that a particular
 // implementation of OpenCL be free to ignore all attributes and the
 // resulting executable binary will produce the same result."
 //
 // However, only AMD GPU targets will emit the corresponding IR
 // attribute.
 //

clang/include/clang/Basic/AttrDocs.td

 }
 
 def AVRInterruptDocs : Documentation {
   let Category = DocCatFunction;
   let Heading = "interrupt (AVR)";
   let Content = [{
 Clang supports the GNU style ``__attribute__((interrupt))`` attribute on
 AVR targets. This attribute may be attached to a function definition and instructs
 the backend to generate appropriate function entry/exit code so that it can be used
 directly as an interrupt service routine.
 
 On the AVR, the hardware globally disables interrupts when an interrupt is executed.
 The first instruction of an interrupt handler declared with this attribute is a SEI
 instruction to re-enable interrupts. See also the signal attribute that
 does not insert a SEI instruction.
   }];
 }
 
 def AVRSignalDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
 Clang supports the GNU style ``__attribute__((signal))`` attribute on
 AVR targets. This attribute may be attached to a function definition and instructs
 the backend to generate appropriate function entry/exit code so that it can be used
 directly as an interrupt service routine.
 

 with different ABI versions supported. For example, a newer version of a class
 could have a different set of data members and thus have a different size. Using
 the ``abi_tag`` attribute, it is possible to have different mangled names for
 a global variable of the class type. Therefore, the old code could keep using
 the old mangled name and the new code will use the new mangled name with tags.
   }];
 }
 
+def BuiltinAliasDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+This attribute is used in the implementation of the C intrinsics.
+It allows the C intrinsic functions to be declared using the names defined
+in target builtins, and still be recognized as clang builtins equivalent to the
+underlying name. For example, ``riscv_vector.h`` declares the function ``vadd``
+with ``__attribute__((clang_builtin_alias(__builtin_rvv_vadd_vv_i8m1)))``.
+This ensures that both functions are recognized as that clang builtin,
+and in the latter case, the choice of which builtin to identify the
+function as can be deferred until after overload resolution.
+
+This attribute can only be used to set up the aliases for certain ARM/RISC-V
+C intrinsic functions; it is intended for use only inside ``arm_*.h`` and
+``riscv_*.h`` and is not a general mechanism for declaring arbitrary aliases
+for clang builtin functions.
+  }];
+}
+
 def PreferredNameDocs : Documentation {
   let Category = DocCatDecl;
   let Content = [{
 The ``preferred_name`` attribute can be applied to a class template, and
 specifies a preferred way of naming a specialization of the template. The
 preferred name will be used whenever the corresponding template specialization
 would otherwise be printed in a diagnostic or similar context.
 

 recognized as that clang builtin, and in the latter case, the choice
 of which builtin to identify the function as can be deferred until
 after overload resolution.
 
 This attribute can only be used to set up the aliases for certain Arm
 intrinsic functions; it is intended for use only inside ``arm_*.h``
 and is not a general mechanism for declaring arbitrary aliases for
 clang builtin functions.
+
+In order to avoid duplicating the attribute definitions for similar
+purpose for other architecture, there is a general form for the
+attribute `clang_builtin_alias`.
   }];
 }
 
 def NoBuiltinDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
 .. Note:: This attribute is not yet fully implemented, it is validated but has
           no effect on the generated code.

clang/include/clang/Basic/DiagnosticSemaKinds.td

def warn_objc_requires_super_protocol : Warning<
   InGroup<DiagGroup<"requires-super-attribute">>;
 def note_protocol_decl : Note<
   "protocol is declared here">;
 def note_protocol_decl_undefined : Note<
   "protocol %0 has no definition">;
 def err_attribute_preferred_name_arg_invalid : Error<
   "argument %0 to 'preferred_name' attribute is not a typedef for "
   "a specialization of %1">;
+def err_attribute_builtin_alias : Error<
+  "%0 attribute can only be applied to a ARM or RISC-V builtin">;
 
 // called-once attribute diagnostics.
 def err_called_once_attribute_wrong_type : Error<
   "'called_once' attribute only applies to function-like parameters">;
 
 def warn_completion_handler_never_called : Warning<
   "%select{|captured }1completion handler is never called">,
   InGroup<CompletionHandler>, DefaultIgnore;

clang/lib/AST/Decl.cpp

 /// \param ConsiderWrapperFunctions If true, we should consider wrapper
 /// functions as their wrapped builtins. This shouldn't be done in general, but
 /// it's useful in Sema to diagnose calls to wrappers based on their semantics.
 unsigned FunctionDecl::getBuiltinID(bool ConsiderWrapperFunctions) const {
   unsigned BuiltinID = 0;
 
   if (const auto *ABAA = getAttr<ArmBuiltinAliasAttr>()) {
     BuiltinID = ABAA->getBuiltinName()->getBuiltinID();
+  } else if (const auto *BAA = getAttr<BuiltinAliasAttr>()) {
+    BuiltinID = BAA->getBuiltinName()->getBuiltinID();
   } else if (const auto *A = getAttr<BuiltinAttr>()) {
     BuiltinID = A->getID();
   }
 
   if (!BuiltinID)
     return 0;
 
   // If the function is marked "overloadable", it has a different mangled name
   // and is not the C library function.
   if (!ConsiderWrapperFunctions && hasAttr<OverloadableAttr>() &&
-      !hasAttr<ArmBuiltinAliasAttr>())
+      (!hasAttr<ArmBuiltinAliasAttr>() && !hasAttr<BuiltinAliasAttr>()))
     return 0;
 
   ASTContext &Context = getASTContext();
   if (!Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID))
     return BuiltinID;
 
   // This function has the name of a known C library
   // function. Determine whether it actually refers to the C library

clang/lib/Sema/SemaDeclAttr.cpp

 
 static bool ArmSveAliasValid(unsigned BuiltinID, StringRef AliasName) {
   switch (BuiltinID) {
   default:
     return false;
 #define GET_SVE_BUILTINS
 #define BUILTIN(name, types, attr) case SVE::BI##name:
 #include "clang/Basic/arm_sve_builtins.inc"
+#undef BUILTIN
     return true;
   }
 }
 
 static void handleArmBuiltinAliasAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   if (!AL.isArgIdent(0)) {
     S.Diag(AL.getLoc(), diag::err_attribute_argument_n_type)
         << AL << 1 << AANT_ArgumentIdentifier;
if ((IsAArch64 && !ArmSveAliasValid(BuiltinID, AliasName)) ||
        !ArmCdeAliasValid(BuiltinID, AliasName))) {
     S.Diag(AL.getLoc(), diag::err_attribute_arm_builtin_alias);
     return;
   }
 
   D->addAttr(::new (S.Context) ArmBuiltinAliasAttr(S.Context, AL, Ident));
 }
 
+static bool RISCVAliasValid(unsigned BuiltinID, StringRef AliasName) {
+  switch (BuiltinID) {
+  default:
+    return false;
+#define BUILTIN(ID, TYPE, ATTRS) case RISCV::BI##ID:
+#include "clang/Basic/BuiltinsRISCV.def"
+#undef BUILTIN
+    return true;
+  }
+}
+
+static void handleBuiltinAliasAttr(Sema &S, Decl *D,
+                                        const ParsedAttr &AL) {
+  if (!AL.isArgIdent(0)) {
+    S.Diag(AL.getLoc(), diag::err_attribute_argument_n_type)
+        << AL << 1 << AANT_ArgumentIdentifier;
+    return;
+  }
+
+  IdentifierInfo *Ident = AL.getArgAsIdent(0)->Ident;
+  unsigned BuiltinID = Ident->getBuiltinID();
+  StringRef AliasName = cast<FunctionDecl>(D)->getIdentifier()->getName();
+
+  bool IsAArch64 = S.Context.getTargetInfo().getTriple().isAArch64();
+  bool IsARM = S.Context.getTargetInfo().getTriple().isARM();
+  bool IsRISCV = S.Context.getTargetInfo().getTriple().isRISCV();
+  if ((IsAArch64 && !ArmSveAliasValid(BuiltinID, AliasName)) ||
+      (IsARM && !ArmMveAliasValid(BuiltinID, AliasName) &&
+       !ArmCdeAliasValid(BuiltinID, AliasName)) ||
+      (IsRISCV && !RISCVAliasValid(BuiltinID, AliasName)) ||
+      (!IsAArch64 && !IsARM && !IsRISCV)) {
+    S.Diag(AL.getLoc(), diag::err_attribute_builtin_alias) << AL;
+    return;
+  }
+
+  D->addAttr(::new (S.Context) BuiltinAliasAttr(S.Context, AL, Ident));
+}
+
 //===----------------------------------------------------------------------===//
 // Checker-specific attribute handlers.
 //===----------------------------------------------------------------------===//
 static bool isValidSubjectOfNSReturnsRetainedAttribute(QualType QT) {
   return QT->isDependentType() || QT->isObjCRetainableType();
 }
 
 static bool isValidSubjectOfNSAttribute(QualType QT) {
static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D,
 
   case ParsedAttr::AT_EnforceTCB:
     handleEnforceTCBAttr<EnforceTCBAttr, EnforceTCBLeafAttr>(S, D, AL);
     break;
 
   case ParsedAttr::AT_EnforceTCBLeaf:
     handleEnforceTCBAttr<EnforceTCBLeafAttr, EnforceTCBAttr>(S, D, AL);
     break;
+
+  case ParsedAttr::AT_BuiltinAlias:
+    handleBuiltinAliasAttr(S, D, AL);
+    break;
   }
 }
 
 /// ProcessDeclAttributeList - Apply all the decl attributes in the specified
 /// attribute list to the specified decl, ignoring any type attributes.
 void Sema::ProcessDeclAttributeList(Scope *S, Decl *D,
                                     const ParsedAttributesView &AttrList,
                                     bool IncludeCXX11Attributes) {

clang/test/CodeGen/RISCV/riscv-attr-builtin-alias-err.c

+// REQUIRES: riscv-registered-target
+// RUN: not %clang_cc1 -triple riscv64 -fsyntax-only -verify \
+// RUN:   -target-feature +experimental-v %s 2>&1 \
+// RUN: | FileCheck %s
+
+#include <riscv_vector.h>
+
+#define __rvv_generic \
+static inline __attribute__((__always_inline__, __nodebug__))
+
+__rvv_generic
+__attribute__((clang_builtin_alias(__builtin_rvv_vadd_vv_i8m1)))
+vint8m1_t vadd_generic (vint8m1_t op0, vint8m1_t op1, size_t op2);
+
+// CHECK: passing 'vint8m2_t' (aka '__rvv_int8m2_t') to parameter of incompatible type 'vint8m1_t'
+vint8m2_t test(vint8m2_t op0, vint8m2_t op1, size_t vl) {
+  vint8m2_t ret = vadd_generic(op0, op1, vl);
+  return ret;
+}

clang/test/CodeGen/RISCV/riscv-attr-builtin-alias.c

+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// REQUIRES: riscv-registered-target
+// RUN: %clang_cc1 -triple riscv64 -emit-llvm -target-feature +experimental-v \
+// RUN:   %s -o - \
+// RUN:   | FileCheck %s
+
+#include <riscv_vector.h>
+
+#define __rvv_generic \
+static inline __attribute__((__always_inline__, __nodebug__))
+
+__rvv_generic
+__attribute__((clang_builtin_alias(__builtin_rvv_vadd_vv_i8m1)))
+vint8m1_t vadd_generic (vint8m1_t op0, vint8m1_t op1, size_t op2);
+
+// CHECK-LABEL: @test(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[OP0_ADDR:%.*]] = alloca <vscale x 8 x i8>, align 1
+// CHECK-NEXT:    [[OP1_ADDR:%.*]] = alloca <vscale x 8 x i8>, align 1
+// CHECK-NEXT:    [[VL_ADDR:%.*]] = alloca i64, align 8
+// CHECK-NEXT:    [[RET:%.*]] = alloca <vscale x 8 x i8>, align 1
+// CHECK-NEXT:    store <vscale x 8 x i8> [[OP0:%.*]], <vscale x 8 x i8>* [[OP0_ADDR]], align 1
+// CHECK-NEXT:    store <vscale x 8 x i8> [[OP1:%.*]], <vscale x 8 x i8>* [[OP1_ADDR]], align 1
+// CHECK-NEXT:    store i64 [[VL:%.*]], i64* [[VL_ADDR]], align 8
+// CHECK-NEXT:    [[TMP0:%.*]] = load <vscale x 8 x i8>, <vscale x 8 x i8>* [[OP0_ADDR]], align 1
+// CHECK-NEXT:    [[TMP1:%.*]] = load <vscale x 8 x i8>, <vscale x 8 x i8>* [[OP1_ADDR]], align 1
+// CHECK-NEXT:    [[TMP2:%.*]] = load i64, i64* [[VL_ADDR]], align 8
+// CHECK-NEXT:    [[TMP3:%.*]] = call <vscale x 8 x i8> @llvm.riscv.vadd.nxv8i8.nxv8i8.i64(<vscale x 8 x i8> [[TMP0]], <vscale x 8 x i8> [[TMP1]], i64 [[TMP2]])
+// CHECK-NEXT:    store <vscale x 8 x i8> [[TMP3]], <vscale x 8 x i8>* [[RET]], align 1
+// CHECK-NEXT:    [[TMP4:%.*]] = load <vscale x 8 x i8>, <vscale x 8 x i8>* [[RET]], align 1
+// CHECK-NEXT:    ret <vscale x 8 x i8> [[TMP4]]
+//
+vint8m1_t test(vint8m1_t op0, vint8m1_t op1, size_t vl) {
+  vint8m1_t ret = vadd_generic(op0, op1, vl);
+  return ret;
+}

clang/test/Misc/pragma-attribute-supported-attributes-list.test

 // CHECK-NEXT: Annotate ()
 // CHECK-NEXT: AnyX86NoCfCheck (SubjectMatchRule_hasType_functionType)
 // CHECK-NEXT: ArcWeakrefUnavailable (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ArmBuiltinAlias (SubjectMatchRule_function)
 // CHECK-NEXT: AssumeAligned (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: Assumption (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: Availability ((SubjectMatchRule_record, SubjectMatchRule_enum, SubjectMatchRule_enum_constant, SubjectMatchRule_field, SubjectMatchRule_function, SubjectMatchRule_namespace, SubjectMatchRule_objc_category, SubjectMatchRule_objc_implementation, SubjectMatchRule_objc_interface, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_objc_protocol, SubjectMatchRule_record, SubjectMatchRule_type_alias, SubjectMatchRule_variable))
 // CHECK-NEXT: BPFPreserveAccessIndex (SubjectMatchRule_record)
+// CHECK-NEXT: BuiltinAlias (SubjectMatchRule_function)
 // CHECK-NEXT: CFAuditedTransfer (SubjectMatchRule_function)
 // CHECK-NEXT: CFConsumed (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: CFICanonicalJumpTable (SubjectMatchRule_function)
 // CHECK-NEXT: CFUnknownTransfer (SubjectMatchRule_function)
 // CHECK-NEXT: CPUDispatch (SubjectMatchRule_function)
 // CHECK-NEXT: CPUSpecific (SubjectMatchRule_function)
 // CHECK-NEXT: CUDAConstant (SubjectMatchRule_variable)
 // CHECK-NEXT: CUDADevice (SubjectMatchRule_function, SubjectMatchRule_variable)