Add -fplugin=name.so option to the driver

[john.brawn]

Summary

This translates to -load name.so in the cc1 command. We can't name the driver option -load, as that means "link against oad", so instead we follow GCC's lead and name the option -fplugin.

[rengolin]

Hi John,

Can you expand a bit more on why you need this, what's the use case, and hopefully attach a test for them?

cheers,
--renato

[john.brawn]

In D12903#247055, @rengolin wrote:

Hi John,

Can you expand a bit more on why you need this, what's the use case, and hopefully attach a test for them?

The use-case is loading llvm plugins that add optimization passes via the PassManagerBuilder::addGlobalExtension mechanism (i.e. just loading the plugin is enough and you don't have to add anything extra to the command-line). Currently you have to do clang -Xclang -load -Xclang plugin.so, and it would be nicer to be able to do clang -load plugin.so.

I'm not sure how to best add a test for this. Maybe I should add another pass to llvm that's built as a plugin like llvm/lib/Transforms/Hello but that uses PassManagerBuilder::addGlobalExtension and have a clang test that runs that?

[rengolin]

In D12903#247075, @john.brawn wrote:

The use-case is loading llvm plugins that add optimization passes via the PassManagerBuilder::addGlobalExtension mechanism (i.e. just loading the plugin is enough and you don't have to add anything extra to the command-line). Currently you have to do clang -Xclang -load -Xclang plugin.so, and it would be nicer to be able to do clang -load plugin.so.

Right, I'm not familiar with that extension, so I can't really give an opinion. Maybe some of the Clang folks could help? Feel free to find the appropriate people and add them as reviewers, so they're aware of it.

I'm not sure how to best add a test for this. Maybe I should add another pass to llvm that's built as a plugin like llvm/lib/Transforms/Hello but that uses PassManagerBuilder::addGlobalExtension and have a clang test that runs that?

Well, since this is Clang, I think just a simple -### test checking that the flag was passed correctly down to CC1 would be enough, as I assume there are tests already for the flag in CC1 mode.

[john.brawn]

Add a test.

[rengolin]

Hi John,

Looks all right to me, but I don't want to approve without someone more familiar with the -load option to agree that this is not an exclusively internal option.

Other than that, I'm happy with it.

cheers,
--renato

[compnerd]

While I agree that this makes the option much nicer to use, it collides with the -l flag. Since it was an internal only option until this point, we should rename it before exposing it at the driver level.

[john.brawn]

In D12903#247934, @compnerd wrote:

While I agree that this makes the option much nicer to use, it collides with the -l flag. Since it was an internal only option until this point, we should rename it before exposing it at the driver level.

Yes, you're right. GCC uses -fplugin=name.so for their plugin interface, which seems reasonable so I'll go with that to avoid having to think up something myself.

[john.brawn]

Rename option from -load to -fplugin.

[compnerd]

Do you know if GCC requires the = or can you do -fplugin name.so ?

[john.brawn]

In D12903#248453, @compnerd wrote:

Do you know if GCC requires the = or can you do -fplugin name.so ?

GCC requires the =, or at least GCC 5.1.0 does when I tested it (you get "gcc: error: name.so: No such file or directory" and "gcc: error: unrecognized command line option '-fplugin'" if you do -fplugin name.so).

[rengolin]

This comment was removed by rengolin.

[compnerd]

Looks fine minus the one thing that needs a quick look at.

include/clang/Driver/Options.td

952

Don't most options of this type use the _EQ suffix? Might be nice to follow the convention.

[thakis]

Is the idea that this loads plugins that link against clang's C++ api? That api isn't considered stable, so not having an "official" flag for this always looked by design to me. Also, C++ doesn't make for a good ABI. Also also, this approach fundamentally doesn't work on Windows.

[rengolin]

In D12903#249406, @thakis wrote:

Also also, this approach fundamentally doesn't work on Windows.

I don't think it's supposed to, anyway. :)

[john.brawn]

In D12903#251361, @rengolin wrote:

In D12903#249406, @thakis wrote:

Also also, this approach fundamentally doesn't work on Windows.

I don't think it's supposed to, anyway. :)

Actually, I think it can work on Windows. Certainly -fplugin=foo.dll works on windows and the dll is correctly loaded, but the problem is building a dll that does something useful. Currently there's no way to build clang/llvm on windows so that LLVM_ENABLE_PLUGINS is true - setting BUILD_SHARED_LIBS sets it to true, but BUILD_SHARED_LIBS doesn't work (lots of linker errors). However I've managed to hack up a build of clang not using BUILD_SHARED_LIBS where it's possible to build plugin dlls that work (by sprinkling __declspec(dllexport) all over the place), so it doesn't look like there's a fundamental problem here. Also maybe it's possible to use llvm-readobj on the intermediate .lib files to generate a list of symbols to export and put that in a .def file for the link step and not have to do any modification to the source files at all, though I haven't been able to get that to work.

Anyway that's a bit of a digression. I've made naming change to OPT_fplugin_EQ, so I'll check that in.

[john.brawn]

Committed in r248378.

include/clang/Driver/Options.td

def fpascal_strings : Flag<["-"], "fpascal-strings">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Recognize and construct Pascal-style string literals">;
 def fpcc_struct_return : Flag<["-"], "fpcc-struct-return">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Override the default ABI to return all structs on the stack">;
 def fpch_preprocess : Flag<["-"], "fpch-preprocess">, Group<f_Group>;
 def fpic : Flag<["-"], "fpic">, Group<f_Group>;
 def fno_pic : Flag<["-"], "fno-pic">, Group<f_Group>;
 def fpie : Flag<["-"], "fpie">, Group<f_Group>;
 def fno_pie : Flag<["-"], "fno-pie">, Group<f_Group>;
+def fplugin : Joined<["-"], "fplugin=">, Group<f_Group>, Flags<[DriverOption]>, MetaVarName<"<dsopath>">,
+  HelpText<"Load the named plugin (dynamic shared object)">;
 def fprofile_arcs : Flag<["-"], "fprofile-arcs">, Group<f_Group>;
 def fno_profile_arcs : Flag<["-"], "fno-profile-arcs">, Group<f_Group>;
 def framework : Separate<["-"], "framework">, Flags<[LinkerInput]>;
 def frandom_seed_EQ : Joined<["-"], "frandom-seed=">, Group<clang_ignored_f_Group>;
 def freg_struct_return : Flag<["-"], "freg-struct-return">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Override the default ABI to return small structs in registers">;
 def frtti : Flag<["-"], "frtti">, Group<f_Group>;
 def : Flag<["-"], "fsched-interblock">, Group<clang_ignored_f_Group>;

lib/Driver/Tools.cpp

#endif
   if (Args.hasArg(options::OPT_fretain_comments_from_system_headers))
     CmdArgs.push_back("-fretain-comments-from-system-headers");
 
   // Forward -fcomment-block-commands to -cc1.
   Args.AddAllArgs(CmdArgs, options::OPT_fcomment_block_commands);
   // Forward -fparse-all-comments to -cc1.
   Args.AddAllArgs(CmdArgs, options::OPT_fparse_all_comments);
 
+  // Turn -fplugin=name.so into -load name.so
+  for (const Arg *A : Args.filtered(options::OPT_fplugin)) {
+    CmdArgs.push_back("-load");
+    CmdArgs.push_back(A->getValue());
+    A->claim();
+  }
+
   // Forward -Xclang arguments to -cc1, and -mllvm arguments to the LLVM option
   // parser.
   Args.AddAllArgValues(CmdArgs, options::OPT_Xclang);
   for (const Arg *A : Args.filtered(options::OPT_mllvm)) {
     A->claim();
 
     // We translate this by hand to the -cc1 argument, since nightly test uses
     // it and developers have been trained to spell it with -mllvm.

test/Driver/fplugin.c

+// Check that all -fplugin arguments are converted to -load
+
+// RUN: %clang -c %s -fplugin=foo.so -### 2>&1                 | FileCheck %s --check-prefix=CHECK1
+// RUN: %clang -c %s -fplugin=foo.so -fplugin=bar.so -### 2>&1 | FileCheck %s --check-prefix=CHECK2
+
+// CHECK1: "-load" "foo.so"
+// CHECK2: "-load" "foo.so" "-load" "bar.so"