[Beignet] [PATCH] GBE: clean llvm module's clone and release.
Yang, Rong R
rong.r.yang at intel.com
Fri Jun 23 05:28:09 UTC 2017
Pushed, thanks.
> -----Original Message-----
> From: Pan, Xiuli
> Sent: Thursday, June 22, 2017 14:54
> To: Yang, Rong R <rong.r.yang at intel.com>; beignet at lists.freedesktop.org
> Cc: Yang, Rong R <rong.r.yang at intel.com>
> Subject: RE: [Beignet] [PATCH] GBE: clean llvm module's clone and release.
>
> LGTM.
>
>
> -----Original Message-----
> From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of
> Yang Rong
> Sent: Thursday, June 22, 2017 14:04
> To: beignet at lists.freedesktop.org
> Cc: Yang, Rong R <rong.r.yang at intel.com>
> 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 <rong.r.yang at intel.com>
> ---
> 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
> Beignet at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/beignet
More information about the Beignet
mailing list