[PATCH] D19181: Map Attribute in the C API.

Amaury SECHET via llvm-commits llvm-commits at lists.llvm.org
Sat Jun 11 17:19:56 PDT 2016


deadalnix marked an inline comment as done.

================
Comment at: include/llvm-c/Core.h:531
@@ +530,3 @@
+ */
+const char *LLVMGetAttributeKindAsString(LLVMAttributeRef A, unsigned *Length);
+
----------------
whitequark wrote:
> Does this allocate? If yes, needs to be mentioned in the docstring I think (etc etc LLVMDisposeString etc etc)
It doesn't.

================
Comment at: tools/llvm-c-test/echo.cpp:800
@@ +799,3 @@
+      for (unsigned k = 0, e = LLVMGetEndAttrKind(); k < e; ++k) {
+        if (LLVMGetAttributeAtIndex(Cur, i, k)) {
+          auto A = LLVMCreateAttribute(Ctx, k, 0);
----------------
whitequark wrote:
> No. This is an incredibly inefficient way to iterate through attributes. A consumer of the C API that attempts to read bitcode would have to do what, at least ~50 FFI calls per function *or* call site just to discover attributes? This will not do.
We discussed that, I'll add getter to get attributes en masse. Can we go incremental here ? This already provides more than the old C API. There is nothing blocking the addition of :

  LLVMGetAttributeCountAtIndex(F, Idx);
  LLVMGetAttributesAtIndex(G, Idx, LLVMAttributeRef *Buf);

Many accessor in the C have the 2 version, and that's fine. Note that the second version require to allocate, which isn't always better, so providing both is fine.

Also, this really doesn't matter as this is not what people commonly do with attribute (unless you often clone module manually ?).


http://reviews.llvm.org/D19181





More information about the llvm-commits mailing list