Add support for attribute in the C API

[deadalnix]

Summary

There is currently no way to assign attribute to the return value and current implementation rely on deprecated code.

The old attribute mechanism is still used for instruction at this stage.

[Wallbraker]

You should put the unrelated commits in another one, they are good clean ups.

I mirror the same comments as in D18733 regarding not directly removing the old functions.

Looks good otherwise, good work!

include/llvm-c/Core.h

1530

While I agree with the change, this is unrelated and should go into another commit.

2063

Unrelated change, in its own commit it is okay.

lib/IR/Core.cpp

927–931

Unrelated change, in its own commit it is okay.

tools/llvm-c-test/echo.cpp

771–777

As I said in D18733 a get number and get KindID at index function would make this less brute force. Alternatively a way to get them all at the same time, instead of "at index" if that is not the way that LLVM does things.

[deadalnix]

Moved NFC changes to their own commit.

[deadalnix]

tools/llvm-c-test/echo.cpp

771–777

I went for the alternative first, namely mapping the attribute set. It was much bigger/more complex, and did not provide any significant advantage in real world use case.

When interacting with attribute, I usually want to check if X has attribute Y, or set attribute Y on X. Rarely do I want to iterate over all attributes (in fact, this test is the only time I ended up doing this).

This test does a lot of wasteful things on purpose in order to grill the C API. I think we are fine here.

[Wallbraker]

tools/llvm-c-test/echo.cpp

771–777

Ah that makes sense and also we discussed this on privately and came to the same conclusion. No need for the extra API.

[rnk]

First, you're breaking compatibility here with old programs using LLVMAttribute. We should ask Jeurgen about that. If we decide that LLVM's underlying attribute handling has changed so drastically that we're OK with a break, then we should delete the LLVMAttribute enum.

Second, you're exposing the C++ enum values of LLVM's attributes. I have a feeling that people will start adding enum attributes in the middle to keep related attributes together. If we want to go this way, where we leverage tablegen instead of maintaining a self-contained mapping the C API layer, then the tablegen records should have manually maintained attribute numbers. We'd either use those as the direct C++ enum values or use them to generate a switch to translate from C to C++. This would be just like protobuf field ids.

[Wallbraker]

In D18727#391285, @rnk wrote:

First, you're breaking compatibility here with old programs using LLVMAttribute. We should ask Jeurgen about that. If we decide that LLVM's underlying attribute handling has changed so drastically that we're OK with a break, then we should delete the LLVMAttribute enum.

Well LLVMAttribute are flags and we have already run out of bits, as per the comment:

/* FIXME: These attributes are currently not included in the C API as
   a temporary measure until the API/ABI impact to the C API is understood
   and the path forward agreed upon.
LLVMSanitizeAddressAttribute = 1ULL << 32,
LLVMStackProtectStrongAttribute = 1ULL<<35,
*/

So something needs to be done.

Second, you're exposing the C++ enum values of LLVM's attributes. I have a feeling that people will start adding enum attributes in the middle to keep related attributes together. If we want to go this way, where we leverage tablegen instead of maintaining a self-contained mapping the C API layer, then the tablegen records should have manually maintained attribute numbers. We'd either use those as the direct C++ enum values or use them to generate a switch to translate from C to C++. This would be just like protobuf field ids.

A translating switch sounds better, especially if the enums on the C++ side change value.

[deadalnix]

Not relevant anymore.

bindings/go/llvm/ir.go

func (v Value) Params() []Value {
   return out
 }
 func (v Value) Param(i int) (rv Value)  { rv.C = C.LLVMGetParam(v.C, C.unsigned(i)); return }
 func (v Value) ParamParent() (rv Value) { rv.C = C.LLVMGetParamParent(v.C); return }
 func (v Value) FirstParam() (rv Value)  { rv.C = C.LLVMGetFirstParam(v.C); return }
 func (v Value) LastParam() (rv Value)   { rv.C = C.LLVMGetLastParam(v.C); return }
 func NextParam(v Value) (rv Value)      { rv.C = C.LLVMGetNextParam(v.C); return }
 func PrevParam(v Value) (rv Value)      { rv.C = C.LLVMGetPreviousParam(v.C); return }
-func (v Value) AddAttribute(a Attribute) {
-  if a >= 1<<32 {
-    panic("attribute value currently unsupported")
-  }
-  C.LLVMAddAttribute(v.C, C.LLVMAttribute(a))
-}
-func (v Value) RemoveAttribute(a Attribute) {
-  if a >= 1<<32 {
-    panic("attribute value currently unsupported")
-  }
-  C.LLVMRemoveAttribute(v.C, C.LLVMAttribute(a))
+func (v Value) AddAttribute(KindID uint) {
+  C.LLVMAddAttribute(v.C, C.uint(KindID))
+func (v Value) RemoveAttribute(KindID uint) {
+  C.LLVMRemoveAttribute(v.C, C.uint(KindID))
 }
-func (v Value) Attribute() Attribute        { return Attribute(C.LLVMGetAttribute(v.C)) }
 func (v Value) SetParamAlignment(align int) { C.LLVMSetParamAlignment(v.C, C.unsigned(align)) }
 
 // Operations on basic blocks
 func (bb BasicBlock) AsValue() (v Value)      { v.C = C.LLVMBasicBlockAsValue(bb.C); return }
 func (v Value) IsBasicBlock() bool            { return C.LLVMValueIsBasicBlock(v.C) != 0 }
 func (v Value) AsBasicBlock() (bb BasicBlock) { bb.C = C.LLVMValueAsBasicBlock(v.C); return }
 func (bb BasicBlock) Parent() (v Value)       { v.C = C.LLVMGetBasicBlockParent(bb.C); return }
 func (v Value) BasicBlocksCount() int         { return int(C.LLVMCountBasicBlocks(v.C)) }

include/llvm-c/Core.h

  *
  * @see DiagnosticInfo::getSeverity()
  */
 LLVMDiagnosticSeverity LLVMGetDiagInfoSeverity(LLVMDiagnosticInfoRef DI);
 
 unsigned LLVMGetMDKindIDInContext(LLVMContextRef C, const char* Name,
                                   unsigned SLen);
 unsigned LLVMGetMDKindID(const char* Name, unsigned SLen);
+const char *LLVMGetStringFromMDKindIDInContext(LLVMContextRef C,
+                                               unsigned KindID,
+                                               size_t *Length);
+const char *LLVMGetStringFromMDKindID(unsigned KindID, size_t *Length);
+
+unsigned LLVMGetEndAttrKind();
+unsigned LLVMGetAttrKindIDInContext(LLVMContextRef C, const char* Name,
+                                    unsigned SLen);
+unsigned LLVMGetAttrKindID(const char* Name, unsigned SLen);
+const char *LLVMGetStringFromAttrKindIDInContext(LLVMContextRef C,
+                                                 unsigned KindID,
+                                                 size_t *Length);
+const char *LLVMGetStringFromAttrKindID(unsigned KindID, size_t *Length);
 
 /**
  * @}
  */
 
 /**
  * @defgroup LLVMCCoreModule Modules
  *

  */
 LLVMBool LLVMIsConstantString(LLVMValueRef c);
 
 /**
  * Get the given constant data sequential as a string.
  *
  * @see ConstantDataSequential::getAsString()
  */
 const char *LLVMGetAsString(LLVMValueRef c, size_t *Length);
 
 /**
  * Create an anonymous ConstantStruct with the specified values.
  *
  * @see llvm::ConstantStruct::getAnon()
  */
 LLVMValueRef LLVMConstStructInContext(LLVMContextRef C,
                                       LLVMValueRef *ConstantVals,

  *
  * @see llvm::Function::setGC()
  */
 void LLVMSetGC(LLVMValueRef Fn, const char *Name);
 
 /**
  * Add an attribute to a function.
  *
- * @see llvm::Function::addAttribute()
+ * @see llvm::Function::addFnAttr()
  */
-void LLVMAddFunctionAttr(LLVMValueRef Fn, LLVMAttribute PA);
+void LLVMAddFunctionAttr(LLVMValueRef Fn, unsigned KindID);
 
 /**
- * Add a target-dependent attribute to a function
+ * Add a target-dependent attribute to a function.
+ *
  * @see llvm::AttrBuilder::addAttribute()
+ * @see llvm::Function::addAttributes()
  */
 void LLVMAddTargetDependentFunctionAttr(LLVMValueRef Fn, const char *A,
                                         const char *V);
 
 /**
- * Obtain an attribute from a function.
- *
- * @see llvm::Function::getAttributes()
+ * Remove an attribute from a function.
  */
-LLVMAttribute LLVMGetFunctionAttr(LLVMValueRef Fn);
+void LLVMRemoveFunctionAttr(LLVMValueRef Fn, unsigned KindID);
 
 /**
- * Remove an attribute from a function.
+ * Check if a function has an attribute.
+ */
+LLVMBool LLVMHasFunctionAttr(LLVMValueRef Fn, unsigned KindID);
+
+/**
+ * Add a return attribute to a function.
  */
-void LLVMRemoveFunctionAttr(LLVMValueRef Fn, LLVMAttribute PA);
+void LLVMAddReturnAttr(LLVMValueRef Fn, unsigned KindID);
+
+/**
+ * Remove a return attribute from a function.
+ */
+void LLVMRemoveReturnAttr(LLVMValueRef Fn, unsigned KindID);
+
+/**
+ * Check if a function's return has an attribute.
+ */
+LLVMBool LLVMHasReturnAttr(LLVMValueRef Fn, unsigned KindID);
 
 /**
  * @defgroup LLVMCCoreValueFunctionParameters Function Parameters
  *
  * Functions in this group relate to arguments/parameters on functions.
  *
  * Functions in this group expect LLVMValueRef instances that correspond
  * to llvm::Function instances.

  */
 LLVMValueRef LLVMGetPreviousParam(LLVMValueRef Arg);
 
 /**
  * Add an attribute to a function argument.
  *
  * @see llvm::Argument::addAttr()
  */
-void LLVMAddAttribute(LLVMValueRef Arg, LLVMAttribute PA);
+void LLVMAddAttribute(LLVMValueRef Arg, unsigned KindID);
 
 /**
  * Remove an attribute from a function argument.
  *
  * @see llvm::Argument::removeAttr()
  */
-void LLVMRemoveAttribute(LLVMValueRef Arg, LLVMAttribute PA);
+void LLVMRemoveAttribute(LLVMValueRef Arg, unsigned KindID);
 
 /**
- * Get an attribute from a function argument.
+ * Remove an attribute from a function argument.
+ *
+ * @see llvm::Argument::hasAttribute()
  */
-LLVMAttribute LLVMGetAttribute(LLVMValueRef Arg);
+LLVMBool LLVMHasAttribute(LLVMValueRef Arg, unsigned KindID);
 
 /**
  * Set the alignment for a function parameter.
  *
  * @see llvm::Argument::addAttr()
  * @see llvm::AttrBuilder::addAlignmentAttr()
  */
 void LLVMSetParamAlignment(LLVMValueRef Arg, unsigned align);

 
 /**
  * Obtain the underlying string from a MDString value.
  *
  * @param V Instance to obtain string from.
  * @param Len Memory address which will hold length of returned string.
  * @return String data in MDString.
  */
 const char *LLVMGetMDString(LLVMValueRef V, unsigned *Length);
 
 /**
  * Obtain the number of operands from an MDNode value.
  *
  * @param V MDNode to get number of operands from.
  * @return Number of operands of the MDNode.
  */
 unsigned LLVMGetMDNodeNumOperands(LLVMValueRef V);

lib/IR/Core.cpp

unsigned LLVMGetMDKindIDInContext(LLVMContextRef C, const char* Name,
                                   unsigned SLen) {
   return unwrap(C)->getMDKindID(StringRef(Name, SLen));
 }
 
 unsigned LLVMGetMDKindID(const char* Name, unsigned SLen) {
   return LLVMGetMDKindIDInContext(LLVMGetGlobalContext(), Name, SLen);
 }
 
+const char *LLVMGetStringFromMDKindIDInContext(LLVMContextRef C,
+                                               unsigned KindID,
+                                               size_t *Length) {
+  SmallVector<StringRef, 16> Names;
+  unwrap(C)->getMDKindNames(Names);
+  StringRef Str = Names[KindID];
+  *Length = Str.size();
+  return Str.data();
+}
+
+const char *LLVMGetStringFromMDKindID(unsigned KindID, size_t *Length) {
+  return LLVMGetStringFromMDKindIDInContext(LLVMGetGlobalContext(),
+                                            KindID, Length);
+}
+
+unsigned LLVMGetEndAttrKind() {
+  return Attribute::AttrKind::EndAttrKinds;
+}
+
+unsigned LLVMGetAttrKindIDInContext(LLVMContextRef C, const char* Name,
+                                    unsigned SLen) {
+  return Attribute::get(*unwrap(C), StringRef(Name, SLen)).getKindAsEnum();
+}
+
+unsigned LLVMGetAttrKindID(const char* Name, unsigned SLen) {
+  return LLVMGetAttrKindIDInContext(LLVMGetGlobalContext(), Name, SLen);
+}
+
+const char *LLVMGetStringFromAttrKindIDInContext(LLVMContextRef C,
+                                                 unsigned KindID,
+                                                 size_t *Length) {
+  auto A = Attribute::get(*unwrap(C), (Attribute::AttrKind)KindID);
+  StringRef Str = A.getKindAsString();
+  *Length = Str.size();
+  return Str.data();
+}
+
+const char *LLVMGetStringFromAttrKindID(unsigned KindID, size_t *Length) {
+  return LLVMGetStringFromAttrKindIDInContext(LLVMGetGlobalContext(),
+                                              KindID, Length);
+}
+
 char *LLVMGetDiagInfoDescription(LLVMDiagnosticInfoRef DI) {
   std::string MsgStorage;
   raw_string_ostream Stream(MsgStorage);
   DiagnosticPrinterRawOStream DP(Stream);
 
   unwrap(DI)->print(DP);
   Stream.flush();
 

 
 LLVMValueRef LLVMGetElementAsConstant(LLVMValueRef C, unsigned idx) {
   return wrap(unwrap<ConstantDataSequential>(C)->getElementAsConstant(idx));
 }
 
 LLVMBool LLVMIsConstantString(LLVMValueRef C) {
   return unwrap<ConstantDataSequential>(C)->isString();
 }
 
 const char *LLVMGetAsString(LLVMValueRef C, size_t *Length) {
   StringRef Str = unwrap<ConstantDataSequential>(C)->getAsString();
   *Length = Str.size();
   return Str.data();
 }
 
 LLVMValueRef LLVMConstArray(LLVMTypeRef ElementTy,
                             LLVMValueRef *ConstantVals, unsigned Length) {
   ArrayRef<Constant*> V(unwrap<Constant>(ConstantVals, Length), Length);
   return wrap(ConstantArray::get(ArrayType::get(unwrap(ElementTy), Length), V));
 }
 

 void LLVMSetGC(LLVMValueRef Fn, const char *GC) {
   Function *F = unwrap<Function>(Fn);
   if (GC)
     F->setGC(GC);
   else
     F->clearGC();
 }
 
-void LLVMAddFunctionAttr(LLVMValueRef Fn, LLVMAttribute PA) {
-  Function *Func = unwrap<Function>(Fn);
-  const AttributeSet PAL = Func->getAttributes();
-  AttrBuilder B(PA);
-  const AttributeSet PALnew =
-    PAL.addAttributes(Func->getContext(), AttributeSet::FunctionIndex,
-                      AttributeSet::get(Func->getContext(),
-                                        AttributeSet::FunctionIndex, B));
-  Func->setAttributes(PALnew);
+void LLVMAddFunctionAttr(LLVMValueRef Fn, unsigned KindID) {
+  unwrap<Function>(Fn)->addFnAttr((Attribute::AttrKind)KindID);
 }
 
 void LLVMAddTargetDependentFunctionAttr(LLVMValueRef Fn, const char *A,
                                         const char *V) {
   Function *Func = unwrap<Function>(Fn);
   AttributeSet::AttrIndex Idx =
     AttributeSet::AttrIndex(AttributeSet::FunctionIndex);
   AttrBuilder B;
 
   B.addAttribute(A, V);
   AttributeSet Set = AttributeSet::get(Func->getContext(), Idx, B);
   Func->addAttributes(Idx, Set);
 }
 
-void LLVMRemoveFunctionAttr(LLVMValueRef Fn, LLVMAttribute PA) {
-  Function *Func = unwrap<Function>(Fn);
-  const AttributeSet PAL = Func->getAttributes();
-  AttrBuilder B(PA);
-  const AttributeSet PALnew =
-    PAL.removeAttributes(Func->getContext(), AttributeSet::FunctionIndex,
-                         AttributeSet::get(Func->getContext(),
-                                           AttributeSet::FunctionIndex, B));
-  Func->setAttributes(PALnew);
+void LLVMRemoveFunctionAttr(LLVMValueRef Fn, unsigned KindID) {
+  unwrap<Function>(Fn)->removeFnAttr((Attribute::AttrKind)KindID);
 }
 
-LLVMAttribute LLVMGetFunctionAttr(LLVMValueRef Fn) {
-  Function *Func = unwrap<Function>(Fn);
-  const AttributeSet PAL = Func->getAttributes();
-  return (LLVMAttribute)PAL.Raw(AttributeSet::FunctionIndex);
+LLVMBool LLVMHasFunctionAttr(LLVMValueRef Fn, unsigned KindID) {
+  return unwrap<Function>(Fn)->hasFnAttribute((Attribute::AttrKind)KindID);
+}
+
+void LLVMAddReturnAttr(LLVMValueRef Fn, unsigned KindID) {
+  unwrap<Function>(Fn)->addAttribute(AttributeSet::ReturnIndex,
+                                     (Attribute::AttrKind)KindID);
+}
+
+void LLVMRemoveReturnAttr(LLVMValueRef Fn, unsigned KindID) {
+  unwrap<Function>(Fn)->removeAttribute(AttributeSet::ReturnIndex,
+                                        (Attribute::AttrKind)KindID);
+}
+
+LLVMBool LLVMHasReturnAttr(LLVMValueRef Fn, unsigned KindID) {
+  return unwrap<Function>(Fn)->hasAttribute(AttributeSet::ReturnIndex,
+                                            (Attribute::AttrKind)KindID);
 }
 
 /*--.. Operations on parameters ............................................--*/
 
 unsigned LLVMCountParams(LLVMValueRef FnRef) {
   // This function is strictly redundant to
   //   LLVMCountParamTypes(LLVMGetElementType(LLVMTypeOf(FnRef)))
   return unwrap<Function>(FnRef)->arg_size();

 LLVMValueRef LLVMGetPreviousParam(LLVMValueRef Arg) {
   Argument *A = unwrap<Argument>(Arg);
   Function::arg_iterator I(A);
   if (I == A->getParent()->arg_begin())
     return nullptr;
   return wrap(&*--I);
 }
 
-void LLVMAddAttribute(LLVMValueRef Arg, LLVMAttribute PA) {
-  Argument *A = unwrap<Argument>(Arg);
-  AttrBuilder B(PA);
-  A->addAttr(AttributeSet::get(A->getContext(), A->getArgNo() + 1,  B));
+void LLVMAddAttribute(LLVMValueRef Arg, unsigned KindID) {
+  unwrap<Argument>(Arg)->addAttr((Attribute::AttrKind)KindID);
 }
 
-void LLVMRemoveAttribute(LLVMValueRef Arg, LLVMAttribute PA) {
-  Argument *A = unwrap<Argument>(Arg);
-  AttrBuilder B(PA);
-  A->removeAttr(AttributeSet::get(A->getContext(), A->getArgNo() + 1,  B));
+void LLVMRemoveAttribute(LLVMValueRef Arg, unsigned KindID) {
+  unwrap<Argument>(Arg)->removeAttr((Attribute::AttrKind)KindID);
 }
 
-LLVMAttribute LLVMGetAttribute(LLVMValueRef Arg) {
-  Argument *A = unwrap<Argument>(Arg);
-  return (LLVMAttribute)A->getParent()->getAttributes().
-    Raw(A->getArgNo()+1);
+LLVMBool LLVMHasAttribute(LLVMValueRef Arg, unsigned KindID) {
+  return unwrap<Argument>(Arg)->hasAttribute((Attribute::AttrKind)KindID);
 }
 
-
 void LLVMSetParamAlignment(LLVMValueRef Arg, unsigned align) {
   Argument *A = unwrap<Argument>(Arg);
   AttrBuilder B;
   B.addAlignmentAttr(align);
   A->addAttr(AttributeSet::get(A->getContext(),A->getArgNo() + 1, B));
 }
 
 /*--.. Operations on basic blocks ..........................................--*/

test/Bindings/llvm-c/invoke.ll

unwind5:                                          ; preds = %unwind3
   br i1 %11, label %catch, label %unwind7
 
 unwind7:                                          ; preds = %unwind5
   resume { i8*, i32 } %4
 }
 
 declare void @_D6object6Object6__ctorFMC6object6ObjectZv(%C6object6Object*)
 
-declare i8* @_d_allocmemory(i64)
+declare noalias i8* @_d_allocmemory(i64)
 
 declare i32 @__sd_eh_personality(i32, i32, i64, i8*, i8*)
 
-declare void @__sd_eh_throw(%C6object9Throwable*)
+declare void @__sd_eh_throw(%C6object9Throwable* nonnull) #0
 
 ; Function Attrs: nounwind readnone
-declare i32 @llvm.eh.typeid.for(i8*) #0
+declare i32 @llvm.eh.typeid.for(i8*) #1
 
-attributes #0 = { nounwind readnone }
+attributes #0 = { noreturn }
+attributes #1 = { nounwind readnone }
 No newline at end of file

tools/llvm-c-test/echo.cpp

static ValueMap clone_params(LLVMValueRef Src, LLVMValueRef Dst) {
   unsigned Count = LLVMCountParams(Src);
   if (Count != LLVMCountParams(Dst))
     report_fatal_error("Parameter count mismatch");
 
   ValueMap VMap;
   if (Count == 0)
     return VMap;
 
+  auto EndAttrKindID = LLVMGetEndAttrKind();
+
   LLVMValueRef SrcFirst = LLVMGetFirstParam(Src);
   LLVMValueRef DstFirst = LLVMGetFirstParam(Dst);
   LLVMValueRef SrcLast = LLVMGetLastParam(Src);
   LLVMValueRef DstLast = LLVMGetLastParam(Dst);
 
   LLVMValueRef SrcCur = SrcFirst;
   LLVMValueRef DstCur = DstFirst;
   LLVMValueRef SrcNext = nullptr;
   LLVMValueRef DstNext = nullptr;
   while (true) {
     const char *Name = LLVMGetValueName(SrcCur);
     LLVMSetValueName(DstCur, Name);
 
+    // Copy attributes
+    for (unsigned k = 0; k < EndAttrKindID; ++k) {
+      if (LLVMHasAttribute(SrcCur, k))
+        LLVMAddAttribute(DstCur, k);
+    }
+
     VMap[SrcCur] = DstCur;
 
     Count--;
     SrcNext = LLVMGetNextParam(SrcCur);
     DstNext = LLVMGetNextParam(DstCur);
     if (SrcNext == nullptr && DstNext == nullptr) {
       if (SrcCur != SrcLast)
         report_fatal_error("SrcLast param does not match End");
FunDecl:
   }
 
   Cur = Begin;
   Next = nullptr;
   while (true) {
     const char *Name = LLVMGetValueName(Cur);
     if (LLVMGetNamedFunction(M, Name))
       report_fatal_error("Function already cloned");
-    LLVMAddFunction(M, Name, LLVMGetElementType(TypeCloner(M).Clone(Cur)));
+    auto Ty = LLVMGetElementType(TypeCloner(M).Clone(Cur));
+    auto F = LLVMAddFunction(M, Name, Ty);
+
+    // Copy attributes
+    for (unsigned k = 0, e = LLVMGetEndAttrKind(); k < e; ++k) {
+      if (LLVMHasFunctionAttr(Cur, k))
+        LLVMAddFunctionAttr(F, k);
+      if (LLVMHasReturnAttr(Cur, k))
+        LLVMAddReturnAttr(F, k);
+    }
 
     Next = LLVMGetNextFunction(Cur);
     if (Next == nullptr) {
       if (Cur != End)
         report_fatal_error("Last function does not match End");
       break;
     }