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

whitequark via llvm-commits llvm-commits at lists.llvm.org
Sat Jun 11 17:26:48 PDT 2016


whitequark added inline comments.

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

================
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);
----------------
deadalnix wrote:
> 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 ?).
`LLVMGetEndAttrKind()` is an API that should not exist; it solves a problem that shouldn't be there in the first place. The cost of making 50 (and growing) FFI calls is much higher than the cost of a single stack allocation. Also, mass getters are not a lot of work.

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

Define "commonly". What you suggest would make reading bitcode and transforming it into some language-specific representation of IR needlessly slow. This actually happens in practice, e.g. Haskell's LLVM-General bindings have a pure representation of IR.


http://reviews.llvm.org/D19181





More information about the llvm-commits mailing list