[Beignet] [PATCH] GBE: clean llvm module's clone and release.

Pan, Xiuli xiuli.pan at intel.com
Thu Jun 22 06:53:47 UTC 2017


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