Pushed, thanks.
> -----Original Message----- > From: Pan, Xiuli > Sent: Thursday, June 22, 2017 14:54 > To: Yang, Rong R <[email protected]>; [email protected] > Cc: Yang, Rong R <[email protected]> > Subject: RE: [Beignet] [PATCH] GBE: clean llvm module's clone and release. > > LGTM. > > > -----Original Message----- > From: Beignet [mailto:[email protected]] On Behalf Of > Yang Rong > Sent: Thursday, June 22, 2017 14:04 > To: [email protected] > Cc: Yang, Rong R <[email protected]> > Subject: [Beignet] [PATCH] GBE: clean llvm module's clone and release. > > There are some changes: > 1. Clone the module before call LLVMLinkModules2, remove other clones for > it. > 2. Don't delete module in function llvmToGen. > 3. Add a function programNewFromLLVMFile so genProgramNewFromLLVM > and buildFromLLVMModule only handle llvm module. Actually, > programNewFromLLVMFile is only used by clCreateProgramWithLLVMIntel, > and I think it is useless, maybe we could delete it at all. > > Signed-off-by: Yang Rong <[email protected]> > --- > backend/src/backend/gen_program.cpp | 5 +- > backend/src/backend/program.cpp | 84 +++++++++++++++++++++------ > ------- > backend/src/backend/program.h | 10 +++- > backend/src/backend/program.hpp | 4 +- > backend/src/llvm/llvm_bitcode_link.cpp | 3 +- > backend/src/llvm/llvm_to_gen.cpp | 19 +------- > backend/src/llvm/llvm_to_gen.hpp | 2 +- > src/cl_gbe_loader.cpp | 5 ++ > src/cl_gbe_loader.h | 1 + > src/cl_program.c | 2 +- > 10 files changed, 77 insertions(+), 58 deletions(-) > > diff --git a/backend/src/backend/gen_program.cpp > b/backend/src/backend/gen_program.cpp > index cfb23fe..bb1d22f 100644 > --- a/backend/src/backend/gen_program.cpp > +++ b/backend/src/backend/gen_program.cpp > @@ -455,7 +455,6 @@ namespace gbe { > } > > static gbe_program genProgramNewFromLLVM(uint32_t deviceID, > - const char *fileName, > const void* module, > const void* llvm_ctx, > const char* asm_file_name, @@ > -475,7 +474,7 @@ > namespace gbe { #ifdef GBE_COMPILER_AVAILABLE > std::string error; > // Try to compile the program > - if (program->buildFromLLVMFile(fileName, module, error, optLevel) == > false) { > + if (program->buildFromLLVMModule(module, error, optLevel) == false) > + { > if (err != NULL && errSize != NULL && stringSize > 0u) { > const size_t msgSize = std::min(error.size(), stringSize-1u); > std::memcpy(err, error.c_str(), msgSize); @@ -598,7 +597,7 @@ > namespace gbe { > acquireLLVMContextLock(); > llvm::Module* module = (llvm::Module*)p->module; > > - if (p->buildFromLLVMFile(NULL, module, error, optLevel) == false) { > + if (p->buildFromLLVMModule(module, error, optLevel) == false) { > if (err != NULL && errSize != NULL && stringSize > 0u) { > const size_t msgSize = std::min(error.size(), stringSize-1u); > std::memcpy(err, error.c_str(), msgSize); diff --git > a/backend/src/backend/program.cpp b/backend/src/backend/program.cpp > index 724058c..740c5c2 100644 > --- a/backend/src/backend/program.cpp > +++ b/backend/src/backend/program.cpp > @@ -40,6 +40,7 @@ > #include "llvm/Support/ManagedStatic.h" > #include "llvm/Transforms/Utils/Cloning.h" > #include "llvm/IR/LLVMContext.h" > +#include "llvm/IRReader/IRReader.h" > #endif > > #include <cstring> > @@ -113,32 +114,17 @@ namespace gbe { > IVAR(OCL_PROFILING_LOG, 0, 0, 1); // Int for different profiling types. > BVAR(OCL_OUTPUT_BUILD_LOG, false); > > - bool Program::buildFromLLVMFile(const char *fileName, > - const void* module, > - std::string &error, > - int optLevel) { > + bool Program::buildFromLLVMModule(const void* module, > + std::string &error, > + int optLevel) { > ir::Unit *unit = new ir::Unit(); > - llvm::Module * cloned_module = NULL; > bool ret = false; > - if(module){ > -#if LLVM_VERSION_MAJOR * 10 + LLVM_VERSION_MINOR >= 38 > - cloned_module = llvm::CloneModule((llvm::Module*)module).release(); > -#else > - cloned_module = llvm::CloneModule((llvm::Module*)module); > -#endif > - } > + > bool strictMath = true; > if (fast_relaxed_math || !OCL_STRICT_CONFORMANCE) > strictMath = false; > -#if LLVM_VERSION_MAJOR * 10 + LLVM_VERSION_MINOR >= 39 > - llvm::Module * linked_module = module ? > llvm::CloneModule((llvm::Module*)module).release() : NULL; > - // Src now will be removed automatically. So clone it. > - if (llvmToGen(*unit, fileName, linked_module, optLevel, strictMath, > OCL_PROFILING_LOG, error) == false) { > -#else > - if (llvmToGen(*unit, fileName, module, optLevel, strictMath, > OCL_PROFILING_LOG, error) == false) { > -#endif > - if (fileName) > - error = std::string(fileName) + " not found"; > + > + if (llvmToGen(*unit, module, optLevel, strictMath, > + OCL_PROFILING_LOG, error) == false) { > delete unit; > return false; > } > @@ -147,13 +133,8 @@ namespace gbe { > if(!unit->getValid()) { > delete unit; //clear unit > unit = new ir::Unit(); > - if(cloned_module){ > - //suppose file exists and llvmToGen will not return false. > - llvmToGen(*unit, fileName, cloned_module, 0, strictMath, > OCL_PROFILING_LOG, error); > - }else{ > - //suppose file exists and llvmToGen will not return false. > - llvmToGen(*unit, fileName, module, 0, strictMath, > OCL_PROFILING_LOG, error); > - } > + //suppose file exists and llvmToGen will not return false. > + llvmToGen(*unit, module, 0, strictMath, OCL_PROFILING_LOG, > + error); > } > if(unit->getValid()){ > std::string error2; > @@ -163,9 +144,6 @@ namespace gbe { > error = error + error2; > } > delete unit; > - if(cloned_module){ > - delete (llvm::Module*) cloned_module; > - } > return ret; > } > > @@ -1094,7 +1072,7 @@ EXTEND_QUOTE: > fclose(asmDumpStream); > } > > - p = gbe_program_new_from_llvm(deviceID, NULL, out_module, > llvm_ctx, > + p = gbe_program_new_from_llvm(deviceID, out_module, llvm_ctx, > dumpASMFileName.empty() ? NULL : > dumpASMFileName.c_str(), > stringSize, err, errSize, optLevel, > options); > if (err != NULL) > @@ -1115,6 +1093,46 @@ EXTEND_QUOTE: > > #ifdef GBE_COMPILER_AVAILABLE > > + static gbe_program programNewFromLLVMFile(uint32_t deviceID, > + const char *fileName, > + size_t string_size, > + char *err, > + size_t *err_size) { > + gbe_program p = NULL; > + if (fileName == NULL) > + return NULL; > + > +#if LLVM_VERSION_MAJOR * 10 + LLVM_VERSION_MINOR >= 39 > + llvm::LLVMContext& c = GBEGetLLVMContext(); #else > + llvm::LLVMContext& c = llvm::getGlobalContext(); #endif #if > +LLVM_VERSION_MAJOR * 10 + LLVM_VERSION_MINOR >= 36 > + // Get the module from its file > + llvm::SMDiagnostic errDiag; > + > + llvm::Module *module = parseIRFile(fileName, errDiag, c).release(); > +#else > + llvm::Module *module = ParseIRFile(fileName, errDiag, c); #endif > + > + int optLevel = 1; > + > + //module will be delete in programCleanLlvmResource > + p = gbe_program_new_from_llvm(deviceID, module, &c, NULL, > + string_size, err, err_size, optLevel, > NULL); > + if (OCL_OUTPUT_BUILD_LOG && err && *err_size) > + llvm::errs() << err << "\n"; > + > + return p; > + } > +#endif > + > + > + > +#ifdef GBE_COMPILER_AVAILABLE > + > static gbe_program programCompileFromSource(uint32_t deviceID, > const char *source, > const char *temp_header_path, @@ > -1502,6 +1520,7 > @@ void releaseLLVMContextLock() } > > GBE_EXPORT_SYMBOL gbe_program_new_from_source_cb > *gbe_program_new_from_source = NULL; > +GBE_EXPORT_SYMBOL gbe_program_new_from_llvm_file_cb > +*gbe_program_new_from_llvm_file = NULL; > GBE_EXPORT_SYMBOL gbe_program_compile_from_source_cb > *gbe_program_compile_from_source = NULL; GBE_EXPORT_SYMBOL > gbe_program_link_program_cb *gbe_program_link_program = NULL; > GBE_EXPORT_SYMBOL gbe_program_check_opt_cb > *gbe_program_check_opt = NULL; @@ -1564,6 +1583,7 @@ namespace gbe > { > CallBackInitializer(void) { > gbe_program_new_from_source = gbe::programNewFromSource; > + gbe_program_new_from_llvm_file = gbe::programNewFromLLVMFile; > gbe_program_compile_from_source = gbe::programCompileFromSource; > gbe_program_link_program = gbe::programLinkProgram; > gbe_program_check_opt = gbe::programCheckOption; diff --git > a/backend/src/backend/program.h b/backend/src/backend/program.h > index e601c97..2017845 100644 > --- a/backend/src/backend/program.h > +++ b/backend/src/backend/program.h > @@ -180,6 +180,15 @@ extern gbe_dup_printfset_cb *gbe_dup_printfset; > typedef void (gbe_output_printf_cb) (void* printf_info, void* buf_addr); > extern gbe_output_printf_cb* gbe_output_printf; > > + > +/*! Create a new program from the llvm file (zero terminated string) */ > +typedef gbe_program (gbe_program_new_from_llvm_file_cb)(uint32_t > deviceID, > + const char *fileName, > + size_t stringSize, > + char *err, > + size_t > +*err_size); extern gbe_program_new_from_llvm_file_cb > +*gbe_program_new_from_llvm_file; > + > /*! Create a new program from the given source code (zero terminated > string) */ typedef gbe_program > (gbe_program_new_from_source_cb)(uint32_t deviceID, > const char *source, @@ > -231,7 +240,6 @@ extern > gbe_program_serialize_to_binary_cb *gbe_program_serialize_to_binary; > > /*! Create a new program from the given LLVM file */ typedef > gbe_program (gbe_program_new_from_llvm_cb)(uint32_t deviceID, > - const char *fileName, > const void *module, > const void *llvm_ctx, > const char > *asm_file_name, diff --git > a/backend/src/backend/program.hpp b/backend/src/backend/program.hpp > index 1aff8b9..4a68e33 100644 > --- a/backend/src/backend/program.hpp > +++ b/backend/src/backend/program.hpp > @@ -305,8 +305,8 @@ namespace gbe { > } > /*! Build a program from a ir::Unit */ > bool buildFromUnit(const ir::Unit &unit, std::string &error); > - /*! Buils a program from a LLVM source code */ > - bool buildFromLLVMFile(const char *fileName, const void* module, > std::string &error, int optLevel); > + /*! Buils a program from a LLVM Module */ > + bool buildFromLLVMModule(const void* module, std::string &error, > + int optLevel); > /*! Buils a program from a OCL string */ > bool buildFromSource(const char *source, std::string &error); > /*! Get size of the global constant arrays */ diff --git > a/backend/src/llvm/llvm_bitcode_link.cpp > b/backend/src/llvm/llvm_bitcode_link.cpp > index 5c6585d..ef56e4c 100644 > --- a/backend/src/llvm/llvm_bitcode_link.cpp > +++ b/backend/src/llvm/llvm_bitcode_link.cpp > @@ -340,7 +340,8 @@ namespace gbe > /* We use beignet's bitcode as dst because it will have a lot of > lazy functions which will not be loaded. */ #if LLVM_VERSION_MAJOR * > 10 + LLVM_VERSION_MINOR >= 39 > - if(LLVMLinkModules2(wrap(clonedLib), wrap(mod))) { > + llvm::Module * linked_module = > llvm::CloneModule((llvm::Module*)mod).release(); > + if(LLVMLinkModules2(wrap(clonedLib), wrap(linked_module))) { > #else > char* errorMsg; > if(LLVMLinkModules(wrap(clonedLib), wrap(mod), > LLVMLinkerDestroySource, &errorMsg)) { diff --git > a/backend/src/llvm/llvm_to_gen.cpp b/backend/src/llvm/llvm_to_gen.cpp > index ceefbbb..8546f73 100644 > --- a/backend/src/llvm/llvm_to_gen.cpp > +++ b/backend/src/llvm/llvm_to_gen.cpp > @@ -288,7 +288,7 @@ namespace gbe > dc->process(diagnostic); > } > > - bool llvmToGen(ir::Unit &unit, const char *fileName,const void* module, > + bool llvmToGen(ir::Unit &unit, const void* module, > int optLevel, bool strictMath, int profiling, std::string > &errors) > { > std::string errInfo; > @@ -296,23 +296,9 @@ namespace gbe > if (OCL_OUTPUT_LLVM_BEFORE_LINK || > OCL_OUTPUT_LLVM_AFTER_LINK || OCL_OUTPUT_LLVM_AFTER_GEN) > o = std::unique_ptr<llvm::raw_fd_ostream>(new > llvm::raw_fd_ostream(fileno(stdout), false)); > > - // Get the module from its file > - llvm::SMDiagnostic Err; > - > Module* cl_mod = NULL; > if (module) { > cl_mod = reinterpret_cast<Module*>(const_cast<void*>(module)); > - } else if (fileName){ > -#if LLVM_VERSION_MAJOR * 10 + LLVM_VERSION_MINOR >= 39 > - llvm::LLVMContext& c = GBEGetLLVMContext(); > -#else > - llvm::LLVMContext& c = llvm::getGlobalContext(); > -#endif > -#if LLVM_VERSION_MAJOR * 10 + LLVM_VERSION_MINOR >= 36 > - cl_mod = parseIRFile(fileName, Err, c).release(); > -#else > - cl_mod = ParseIRFile(fileName, Err, c); > -#endif > } > > if (!cl_mod) return false; > @@ -335,8 +321,7 @@ namespace gbe > /* Before do any thing, we first filter in all CL functions in bitcode. > */ > /* Also set unit's pointer size in runBitCodeLinker */ > M.reset(runBitCodeLinker(cl_mod, strictMath, unit)); > - if (!module) > - delete cl_mod; > + > if (M.get() == 0) > return true; > > diff --git a/backend/src/llvm/llvm_to_gen.hpp > b/backend/src/llvm/llvm_to_gen.hpp > index 9025852..73e8819 100644 > --- a/backend/src/llvm/llvm_to_gen.hpp > +++ b/backend/src/llvm/llvm_to_gen.hpp > @@ -35,7 +35,7 @@ namespace gbe { > > /*! Convert the LLVM IR code to a GEN IR code, > optLevel 0 equal to clang -O1 and 1 equal to clang -O2*/ > - bool llvmToGen(ir::Unit &unit, const char *fileName, const void* module, > + bool llvmToGen(ir::Unit &unit, const void* module, > int optLevel, bool strictMath, int profiling, std::string > &errors); #if > LLVM_VERSION_MAJOR * 10 + LLVM_VERSION_MINOR >= 39 > extern llvm::LLVMContext& GBEGetLLVMContext(); diff --git > a/src/cl_gbe_loader.cpp b/src/cl_gbe_loader.cpp index f190b0d..0379b3e > 100644 > --- a/src/cl_gbe_loader.cpp > +++ b/src/cl_gbe_loader.cpp > @@ -24,6 +24,7 @@ > > //function pointer from libgbe.so > gbe_program_new_from_source_cb > *compiler_program_new_from_source = NULL; > +gbe_program_new_from_llvm_file_cb > *compiler_program_new_from_llvm_file > += NULL; > gbe_program_compile_from_source_cb > *compiler_program_compile_from_source = NULL; > gbe_program_new_gen_program_cb > *compiler_program_new_gen_program = NULL; > gbe_program_link_program_cb *compiler_program_link_program = NULL; > @@ -298,6 +299,10 @@ struct GbeLoaderInitializer > if (compiler_program_new_from_source == NULL) > return; > > + compiler_program_new_from_llvm_file = > *(gbe_program_new_from_llvm_file_cb **)dlsym(dlhCompiler, > "gbe_program_new_from_llvm_file"); > + if (compiler_program_new_from_llvm_file == NULL) > + return; > + > compiler_program_compile_from_source = > *(gbe_program_compile_from_source_cb **)dlsym(dlhCompiler, > "gbe_program_compile_from_source"); > if (compiler_program_compile_from_source == NULL) > return; > diff --git a/src/cl_gbe_loader.h b/src/cl_gbe_loader.h index df885d2..df85f1e > 100644 > --- a/src/cl_gbe_loader.h > +++ b/src/cl_gbe_loader.h > @@ -25,6 +25,7 @@ > extern "C" { > #endif > extern gbe_program_new_from_source_cb > *compiler_program_new_from_source; > +extern gbe_program_new_from_llvm_file_cb > +*compiler_program_new_from_llvm_file; > extern gbe_program_compile_from_source_cb > *compiler_program_compile_from_source; > extern gbe_program_new_gen_program_cb > *compiler_program_new_gen_program; > extern gbe_program_link_program_cb *compiler_program_link_program; > diff --git a/src/cl_program.c b/src/cl_program.c index bb96d98..faa3572 > 100644 > --- a/src/cl_program.c > +++ b/src/cl_program.c > @@ -458,7 +458,7 @@ cl_program_create_from_llvm(cl_context ctx, > goto error; > } > > - program->opaque = compiler_program_new_from_llvm(ctx->devices[0]- > >device_id, file_name, NULL, NULL, NULL, program->build_log_max_sz, > program->build_log, &program->build_log_sz, 1, NULL); > + program->opaque = > + compiler_program_new_from_llvm_file(ctx->devices[0]->device_id, > + file_name, program->build_log_max_sz, program->build_log, > + &program->build_log_sz); > if (UNLIKELY(program->opaque == NULL)) { > err = CL_INVALID_PROGRAM; > goto error; > -- > 2.7.4 > > _______________________________________________ > Beignet mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/beignet _______________________________________________ Beignet mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/beignet
