[PATCH] D31898: Introduce libLTO C APIs to target the "resolution-based" new LTO API

[Jakob Bornecrantz via Phabricator via llvm-commits]

Wallbraker added inline comments.


================
Comment at: llvm/include/llvm-c/lto2.h:61
+  HiddenVisibility,      ///< The GV is hidden
+  ProtectedVisibility    ///< The GV is protected
+};
----------------
These will need to be prefixed with LTO or something like that. See the LLVM-C enums.




================
Comment at: llvm/include/llvm-c/lto2.h:77
+ * generated */
+typedef void (*ObjectReadyCallback)(lto_output_t, void *context);
+
----------------
Add to the comment a note about threading. If you use multiple threads does the callback happen on different threads?

Is there a way to avoid using a callback?


================
Comment at: llvm/include/llvm-c/lto2.h:109
+/// Helper function to create a string parameter.
+lto_config_param_t lto_make_str_param(lto_config_params_tag Tag, const char *);
+
----------------
This API feels very un LLVM-Cy, maybe make the lto_config_t object opaque and have functions to add params to it?


================
Comment at: llvm/include/llvm-c/lto2.h:207
+ * supposed to be called once per instance. The instance can't be reused and
+ * should be destroyed after this call returns.
+ */
----------------
Why not just destroy it automatically?


https://reviews.llvm.org/D31898