[PATCH] D35258: [Plugins] Add a slim plugin API to work together with the new PM

Chandler Carruth via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 5 01:51:29 PDT 2018


chandlerc accepted this revision.
chandlerc added a comment.
This revision is now accepted and ready to land.

LGTM with doc fixes and `Twine` fixes below. Really excited about this!

Thanks for the ton of work getting us from zero plugins to rich plugins!



================
Comment at: include/llvm/Passes/PassPlugin.h:37
+extern "C" {
+/// Information about the plugin required to load its passes
+struct PassPluginLibraryInfo {
----------------
Maybe expand this a bit to explain this defines the core interface used to interact with plugins but is primarily for plugin implementors (as opposed to plugin users) and point users to the class below that loads and wraps the plugin?


================
Comment at: include/llvm/Passes/PassPlugin.h:53
+
+/// %Metadata for a loaded pass plugin
+class PassPlugin {
----------------
`%Metadata`?


================
Comment at: lib/Passes/PassPlugin.cpp:38-40
+        "Wrong API version on plugin '" + Filename + "'. Got version " +
+            std::to_string(P.Info.APIVersion) + ", supported version is " +
+            std::to_string(LLVM_PLUGIN_API_VERSION) + ".",
----------------
chandlerc wrote:
> Here in particular using the `Twine` composition is likely especially good. You should be able to either directly pass the C-strings or build StringRef's around them and avoid any allocation at this layer.
Rather than `std::to_string`, you can just call `Twine` on integers.

In fact, I think the operator+ overload will do the right thing here, and you can just add these together and it will do twine-based concatenation.


Repository:
  rL LLVM

https://reviews.llvm.org/D35258





More information about the llvm-commits mailing list