Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Load plugins in-process. #78

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Load plugins in-process. #78

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/driver/AmdCompiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ class AMDGPUCompiler : public Compiler {
ArgStringList GetJobArgsFitered(const Command& job);
// Parse -mllvm options
bool ParseLLVMOptions(const std::vector<std::string>& options);
bool LoadPlugins(const std::vector<std::string>& plugins);
bool PrepareCompiler(CompilerInstance& clang, const Command& job);
bool PrepareAssembler(AssemblerInvocation &Opts, const Command& job);
bool ExecuteCompiler(CompilerInstance& clang, BackendAction action);
Expand Down Expand Up @@ -772,6 +773,19 @@ bool AMDGPUCompiler::ParseLLVMOptions(const std::vector<std::string>& options) {
return true;
}

bool AMDGPUCompiler::LoadPlugins(const std::vector<std::string>& plugins) {
if (plugins.empty()) { return true; }
// Load any requested plugins.
for (const auto &path : plugins) {
std::string error;
if (llvm::sys::DynamicLibrary::LoadLibraryPermanently(path.c_str(), &error)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like LLVM is able only to load library permanently and not to unload:

Note: there is currently no interface for temporarily loading a library

But the in-process mode of the compiler implies that it is always in memory and might live as long as the Runtime by which it's invoked. It also means that compilation might be called (and is called) multiple times for different sources and with different options, including options for loading plugins. I believe that we should load plugins per one compilation (clang::driverCompilation class instance).

So we need a solution for unloading plugin library. What do you think?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I'm unsure if there is a specific reason that there is no interface for temporarily loading a library. It looks like libraries are unloaded when llvm::llvm_shutdown is called. AFAICS with AMDOCL in-process llvm_shutdown is never called. We cannot call llvm_shutdown after each compilation because it will effect more than just loaded libraries. So I guess one option would be to re-initialize LLVM before each compilation so that llvm_shutdown can be called.

I don't think there is a good solution for unloading a plugin library without patching LLVM. I tried something like this:

void DynamicLibrary::UnloadLibraries() {
  SmartScopedLock<true> Lock(*SymbolsMutex);
  // 
  // We would also need to Lock ManagedStaticMutex
  // here I guess. 
  //
  // Unload dynamic libraries by destroying HandleSet.
  if (OpenedHandles.isConstructed())
    OpenedHandles.destroy();
}

The call to ::dlclose doesn't necessarily unload the library, because it seems like I can still use the plugin for the next clang::driverCompilation even without reloading it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about switching off in-process in case of loading plugin(s) (as long as it seems there is no solution for temporary loading plugins from libraries, at least yet)?

Btw, llvm_shutdown needs LLVM (re)initialization, but it also doesn't work since llvm_shutdown was designed to be called just before application termination and actually does not imply further work with LLVM.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about switching off in-process in case of loading plugin(s)

I like this idea.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, let's leave the change with in-process for a while, and make instead switching in-process off in case of loading plugin (corresponding options are found) and switching in-process back on after finishing compilation. In case of in-process off do nothing.

diags.Report(diag::err_fe_unable_to_load_plugin) << path << error;
return false;
}
}
return true;
}

void AMDGPUCompiler::ResetOptionsToDefault() {
cl::ResetAllOptionOccurrences();
for (auto SC : cl::getRegisteredSubcommands()) {
Expand All @@ -791,6 +805,7 @@ bool AMDGPUCompiler::PrepareCompiler(CompilerInstance& clang, const Command& job
const_cast<const char**>(args.data()),
const_cast<const char**>(args.data()) + args.size(),
clang.getDiagnostics())) { return false; }
if (!LoadPlugins(clang.getFrontendOpts().Plugins)) { return false; }
if (!ParseLLVMOptions(clang.getFrontendOpts().LLVMArgs)) { return false; }
return true;
}
Expand Down