diff --git a/interpreter/cling/lib/Interpreter/BackendPasses.cpp b/interpreter/cling/lib/Interpreter/BackendPasses.cpp index 5311ba274c7dd..2e4bcd44c00e3 100644 --- a/interpreter/cling/lib/Interpreter/BackendPasses.cpp +++ b/interpreter/cling/lib/Interpreter/BackendPasses.cpp @@ -259,51 +259,72 @@ namespace { // declarations. This reduces the amount of emitted symbols. class ReuseExistingWeakSymbols : public ModulePass { static char ID; + cling::IncrementalJIT &m_JIT; - bool runOnGlobal(GlobalValue& GV) { - if (GV.isDeclaration()) - return false; // no change. - - // GV is a definition. - + bool shouldRemoveGlobalDefinition(GlobalValue& GV) { + // Existing *weak* symbols can be re-used thanks to ODR. llvm::GlobalValue::LinkageTypes LT = GV.getLinkage(); if (!GV.isDiscardableIfUnused(LT) || !GV.isWeakForLinker(LT)) return false; - // Find the symbol in JIT or shared libraries (without auto-loading). - std::string Name = GV.getName().str(); - if ( + // Find the symbol as existing, previously compiled symbol in the JIT... + if (m_JIT.doesSymbolAlreadyExist(GV.getName())) + return true; + + // ...or in shared libraries (without auto-loading). + std::string Name = GV.getName().str(); #if !defined(_WIN32) - llvm::sys::DynamicLibrary::SearchForAddressOfSymbol(Name) + return llvm::sys::DynamicLibrary::SearchForAddressOfSymbol(Name); #else - platform::DLSym(Name) + return platform::DLSym(Name); #endif - ) { + } + + bool runOnVar(GlobalVariable& GV) { #if !defined(_WIN32) - // Heuristically, Windows cannot handle cross-library variables; they - // must be library-local. - if (auto *Var = dyn_cast(&GV)) { - Var->setInitializer(nullptr); // make this a declaration - } else + // Heuristically, Windows cannot handle cross-library variables; they + // must be library-local. + + if (GV.isDeclaration()) + return false; // no change. + if (shouldRemoveGlobalDefinition(GV)) { + GV.setInitializer(nullptr); // make this a declaration + return true; // a change! + } +#endif + return false; // no change. + } + + bool runOnFunc(Function& Func) { + if (Func.isDeclaration()) + return false; // no change. +#ifndef _WIN32 + // MSVC's stdlib gets symbol issues; i.e. apparently: JIT all or none. + if (Func.getInstructionCount() < 50) { + // This is a small function. Keep its definition to retain it for + // inlining: the cost for JITting it is small, and the likelihood + // that the call will be inlined is high. + return false; + } #endif - if (auto *Func = dyn_cast(&GV)) { - Func->deleteBody(); // make this a declaration - } + if (shouldRemoveGlobalDefinition(Func)) { + Func.deleteBody(); // make this a declaration return true; // a change! } - return false; + return false; // no change. } public: - ReuseExistingWeakSymbols() : - ModulePass(ID) {} + ReuseExistingWeakSymbols(IncrementalJIT &JIT) : + ModulePass(ID), m_JIT(JIT) {} bool runOnModule(Module &M) override { bool ret = false; + // FIXME: use SymbolLookupSet, rather than looking up symbol by symbol. for (auto &&F: M) - ret |= runOnGlobal(F); + ret |= runOnFunc(F); for (auto &&G: M.globals()) - ret |= runOnGlobal(G); + ret |= runOnVar(G); return ret; } }; @@ -313,8 +334,9 @@ char ReuseExistingWeakSymbols::ID = 0; BackendPasses::BackendPasses(const clang::CodeGenOptions &CGOpts, - llvm::TargetMachine& TM): + IncrementalJIT &JIT, llvm::TargetMachine& TM): m_TM(TM), + m_JIT(JIT), m_CGOpts(CGOpts) {} @@ -392,7 +414,7 @@ void BackendPasses::CreatePasses(llvm::Module& M, int OptLevel) m_MPM[OptLevel]->add(new KeepLocalGVPass()); m_MPM[OptLevel]->add(new PreventLocalOptPass()); m_MPM[OptLevel]->add(new WeakTypeinfoVTablePass()); - m_MPM[OptLevel]->add(new ReuseExistingWeakSymbols()); + m_MPM[OptLevel]->add(new ReuseExistingWeakSymbols(m_JIT)); // The function __cuda_module_ctor and __cuda_module_dtor will just generated, // if a CUDA fatbinary file exist. Without file path there is no need for the diff --git a/interpreter/cling/lib/Interpreter/BackendPasses.h b/interpreter/cling/lib/Interpreter/BackendPasses.h index 87d633c982812..11f753fad68da 100644 --- a/interpreter/cling/lib/Interpreter/BackendPasses.h +++ b/interpreter/cling/lib/Interpreter/BackendPasses.h @@ -44,12 +44,13 @@ namespace cling { std::array, 4> m_FPM; llvm::TargetMachine& m_TM; + IncrementalJIT &m_JIT; const clang::CodeGenOptions &m_CGOpts; void CreatePasses(llvm::Module& M, int OptLevel); public: - BackendPasses(const clang::CodeGenOptions &CGOpts, + BackendPasses(const clang::CodeGenOptions &CGOpts, IncrementalJIT &JIT, llvm::TargetMachine& TM); ~BackendPasses(); diff --git a/interpreter/cling/lib/Interpreter/IncrementalExecutor.cpp b/interpreter/cling/lib/Interpreter/IncrementalExecutor.cpp index c32103ab783ea..592d67975a813 100644 --- a/interpreter/cling/lib/Interpreter/IncrementalExecutor.cpp +++ b/interpreter/cling/lib/Interpreter/IncrementalExecutor.cpp @@ -55,7 +55,7 @@ IncrementalExecutor::IncrementalExecutor(clang::DiagnosticsEngine& /*diags*/, llvm_unreachable("Propagate this error and exit gracefully"); } - m_BackendPasses.reset(new BackendPasses(CI.getCodeGenOpts(), m_JIT->getTargetMachine())); + m_BackendPasses.reset(new BackendPasses(CI.getCodeGenOpts(), *m_JIT, m_JIT->getTargetMachine())); } IncrementalExecutor::~IncrementalExecutor() {} diff --git a/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp b/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp index f6aa0265f662f..e736c0918bd46 100644 --- a/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp +++ b/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp @@ -504,6 +504,10 @@ llvm::Error IncrementalJIT::removeModule(const Transaction& T) { m_ResourceTrackers.erase(&T); if (Error Err = RT->remove()) return Err; + auto iMod = m_CompiledModules.find(T.m_CompiledModule); + if (iMod != m_CompiledModules.end()) + m_CompiledModules.erase(iMod); + return llvm::Error::success(); } @@ -550,7 +554,7 @@ IncrementalJIT::addOrReplaceDefinition(StringRef Name, return KnownAddr; } -void* IncrementalJIT::getSymbolAddress(StringRef Name, bool IncludeHostSymbols) { +void* IncrementalJIT::getSymbolAddress(StringRef Name, bool IncludeHostSymbols){ std::unique_lock G(SkipHostProcessLookup, std::defer_lock); if (!IncludeHostSymbols) G.lock(); @@ -577,4 +581,13 @@ void* IncrementalJIT::getSymbolAddress(StringRef Name, bool IncludeHostSymbols) return jitTargetAddressToPointer(Symbol->getAddress()); } +bool IncrementalJIT::doesSymbolAlreadyExist(StringRef UnmangledName) { + auto Name = Jit->mangle(UnmangledName); + for (auto &&M: m_CompiledModules) { + if (M.first->getNamedValue(Name)) + return true; + } + return false; +} + } // namespace cling diff --git a/interpreter/cling/lib/Interpreter/IncrementalJIT.h b/interpreter/cling/lib/Interpreter/IncrementalJIT.h index 6d067a6d5b7ff..c05458c241f4f 100644 --- a/interpreter/cling/lib/Interpreter/IncrementalJIT.h +++ b/interpreter/cling/lib/Interpreter/IncrementalJIT.h @@ -78,6 +78,10 @@ class IncrementalJIT { /// should include symbols from the host process (via dlsym) or not. void* getSymbolAddress(llvm::StringRef Name, bool IncludeHostSymbols); + /// @brief Check whether the JIT already has emitted or knows how to emit + /// a symbol based on its IR name (as coming from clang's mangler). + bool doesSymbolAlreadyExist(llvm::StringRef UnmangledName); + /// Inject a symbol with a known address. Name is not linker mangled, i.e. /// as known by the IR. llvm::JITTargetAddress addOrReplaceDefinition(llvm::StringRef Name,