[Beignet] [PATCH v2] Runtime: implement the get build log function and fix one build error check issue.

Zhigang Gong zhigang.gong at intel.com
Wed Nov 27 00:04:43 PST 2013


According to spec, we need to support CL_PROGRAM_BUILD_LOG which is
used to get the build log of a cl kernel. And we also need to check
whether a build failure is a generic build fail or a build option
error. This commit also fix the piglit case:
API/clBuildProgram.

Another change in this commit is that it reroute all the output
of the clang excution to internal buffer and don't print to the
console directly. If the user want to get the detail build log,
the CL_PROGRAM_BUILD_LOG could be used.

v2: include both clang error messages and the llvm-to-gen error
messages. Also refine the checking for the error buffer parameter.
If there is no error buffer specified, always flush the build log
to llvm::errs().

Signed-off-by: Zhigang Gong <zhigang.gong at intel.com>
---
 backend/src/backend/program.cpp |   73 +++++++++++++++++++++++++++++----------
 src/cl_api.c                    |    5 +--
 src/cl_program.c                |   13 ++++---
 src/cl_program.h                |    3 ++
 4 files changed, 70 insertions(+), 24 deletions(-)

diff --git a/backend/src/backend/program.cpp b/backend/src/backend/program.cpp
index 093febc..8e80bbb 100644
--- a/backend/src/backend/program.cpp
+++ b/backend/src/backend/program.cpp
@@ -464,7 +464,8 @@ namespace gbe {
     GBE_SAFE_DELETE(program);
   }
 
-  static void buildModuleFromSource(const char* input, const char* output, std::string options) {
+  static bool buildModuleFromSource(const char* input, const char* output, std::string options,
+                                    size_t stringSize, char *err, size_t *errSize) {
     // Arguments to pass to the clang frontend
     vector<const char *> args;
     bool bOpt = true;
@@ -516,24 +517,26 @@ namespace gbe {
     args.push_back(input);
 
     // The compiler invocation needs a DiagnosticsEngine so it can report problems
+    std::string ErrorString;
+    llvm::raw_string_ostream ErrorInfo(ErrorString);
+    llvm::IntrusiveRefCntPtr<clang::DiagnosticOptions> DiagOpts = new clang::DiagnosticOptions();
+    DiagOpts->ShowCarets = false;
 #if LLVM_VERSION_MINOR <= 1
     args.push_back("-triple");
     args.push_back("ptx32");
 
     clang::TextDiagnosticPrinter *DiagClient =
-                             new clang::TextDiagnosticPrinter(llvm::errs(), clang::DiagnosticOptions());
+                             new clang::TextDiagnosticPrinter(ErrorInfo, *DiagOpts)
     llvm::IntrusiveRefCntPtr<clang::DiagnosticIDs> DiagID(new clang::DiagnosticIDs());
     clang::DiagnosticsEngine Diags(DiagID, DiagClient);
 #else
     args.push_back("-ffp-contract=off");
 
-    llvm::IntrusiveRefCntPtr<clang::DiagnosticOptions> DiagOpts = new clang::DiagnosticOptions();
     clang::TextDiagnosticPrinter *DiagClient =
-                             new clang::TextDiagnosticPrinter(llvm::errs(), &*DiagOpts);
+                             new clang::TextDiagnosticPrinter(ErrorInfo, &*DiagOpts);
     llvm::IntrusiveRefCntPtr<clang::DiagnosticIDs> DiagID(new clang::DiagnosticIDs());
     clang::DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagClient);
 #endif /* LLVM_VERSION_MINOR <= 1 */
-
     // Create the compiler invocation
     llvm::OwningPtr<clang::CompilerInvocation> CI(new clang::CompilerInvocation);
     clang::CompilerInvocation::CreateFromArgs(*CI,
@@ -548,10 +551,12 @@ namespace gbe {
 #if LLVM_VERSION_MINOR <= 2
     Clang.createDiagnostics(args.size(), &args[0]);
 #else
-    Clang.createDiagnostics();
+    Clang.createDiagnostics(DiagClient, false);
 #endif /* LLVM_VERSION_MINOR <= 2 */
+
+    Clang.getDiagnosticOpts().ShowCarets = false;
     if (!Clang.hasDiagnostics())
-      return;
+      return false;
 
     // Set Language
     clang::LangOptions & lang_opts = Clang.getLangOpts();
@@ -573,23 +578,42 @@ namespace gbe {
     // Create an action and make the compiler instance carry it out
     llvm::OwningPtr<clang::CodeGenAction> Act(new clang::EmitLLVMOnlyAction());
     auto retVal = Clang.ExecuteAction(*Act);
+
+    if (err != NULL) {
+      GBE_ASSERT(errSize != NULL);
+      *errSize = ErrorString.copy(err, stringSize - 1, 0);
+      ErrorString.clear();
+    } else {
+      // flush the error messages to the errs() if there is no
+      // error string buffer.
+      llvm::errs() << ErrorString;
+    }
     if (!retVal)
-      return;
+      return false;
 
     llvm::Module *module = Act->takeModule();
 
-    std::string ErrorInfo;
 #if (LLVM_VERSION_MAJOR == 3) && (LLVM_VERSION_MINOR > 3)
     auto mode = llvm::sys::fs::F_Binary;
 #else
     auto mode = llvm::raw_fd_ostream::F_Binary;
 #endif
-    llvm::raw_fd_ostream OS(output, ErrorInfo, mode);
+    llvm::raw_fd_ostream OS(output, ErrorString, mode);
     //still write to temp file for code simply, otherwise need add another function.
     //because gbe_program_new_from_llvm also be used by cl_program_create_from_llvm, can't be removed
     //TODO: Pass module to llvmToGen, if use module, should return Act and use OwningPtr out of this funciton
     llvm::WriteBitcodeToFile(module, OS);
+    if (err != NULL && *errSize < stringSize - 1 && ErrorString.size() > 0) {
+      size_t errLen;
+      errLen = ErrorString.copy(err + *errSize, stringSize - *errSize - 1, 0);
+      *errSize += errLen;
+    } else if (err == NULL) {
+      // flush the error messages to the errs() if there is no
+      // error string buffer.
+      llvm::errs() << ErrorString;
+    }
     OS.close();
+    return true;
   }
 
   extern std::string ocl_stdlib_str;
@@ -640,15 +664,28 @@ namespace gbe {
     fwrite(source, strlen(source), 1, clFile);
     fclose(clFile);
 
-    buildModuleFromSource(clName.c_str(), llName.c_str(), clOpt.c_str());
-    remove(clName.c_str());
-
+    gbe_program p;
+    if (buildModuleFromSource(clName.c_str(), llName.c_str(), clOpt.c_str(),
+                              stringSize, err, errSize)) {
     // Now build the program from llvm
-    static std::mutex gbe_mutex;
-    gbe_mutex.lock();
-    gbe_program p = gbe_program_new_from_llvm(llName.c_str(), stringSize, err, errSize);
-    gbe_mutex.unlock();
-    remove(llName.c_str());
+      static std::mutex gbe_mutex;
+      gbe_mutex.lock();
+      size_t clangErrSize = 0;
+      if (err != NULL) {
+        GBE_ASSERT(errSize != NULL);
+        stringSize -= *errSize;
+        err += *errSize;
+        clangErrSize = *errSize;
+      }
+      p = gbe_program_new_from_llvm(llName.c_str(), stringSize,
+                                    err, errSize);
+      if (err != NULL)
+        *errSize += clangErrSize;
+      gbe_mutex.unlock();
+      remove(llName.c_str());
+    } else
+      p = NULL;
+    remove(clName.c_str());
     return p;
   }
 
diff --git a/src/cl_api.c b/src/cl_api.c
index 59c47d3..0978129 100644
--- a/src/cl_api.c
+++ b/src/cl_api.c
@@ -975,8 +975,9 @@ clGetProgramBuildInfo(cl_program             program,
 
     FILL_GETINFO_RET (char, (strlen(ret_str)+1), ret_str, CL_SUCCESS);
   } else if (param_name == CL_PROGRAM_BUILD_LOG) {
-    // TODO: need to add logs in backend when compiling.
-    FILL_GETINFO_RET (char, (strlen(ret_str)+1), ret_str, CL_SUCCESS);
+    FILL_GETINFO_RET (char, program->build_log_sz + 1, program->build_log, CL_SUCCESS);
+    if (param_value_size_ret)
+      *param_value_size_ret = program->build_log_sz + 1;
   } else {
     return CL_INVALID_VALUE;
   }
diff --git a/src/cl_program.c b/src/cl_program.c
index df2f1e0..d6d68c0 100644
--- a/src/cl_program.c
+++ b/src/cl_program.c
@@ -109,7 +109,9 @@ cl_program_new(cl_context ctx)
   p->ref_n = 1;
   p->magic = CL_MAGIC_PROGRAM_HEADER;
   p->ctx = ctx;
-
+  p->build_log = calloc(200, sizeof(char));
+  if (p->build_log)
+    p->build_log_max_sz = 200;
   /* The queue also belongs to its context */
   cl_context_add_ref(ctx);
 
@@ -223,7 +225,7 @@ cl_program_create_from_llvm(cl_context ctx,
   INVALID_VALUE_IF (file_name == NULL);
 
   program = cl_program_new(ctx);
-  program->opaque = gbe_program_new_from_llvm(file_name, 0, NULL, NULL);
+  program->opaque = gbe_program_new_from_llvm(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;
@@ -324,9 +326,12 @@ cl_program_build(cl_program p, const char *options)
   }
 
   if (p->source_type == FROM_SOURCE) {
-    p->opaque = gbe_program_new_from_source(p->source, 0, options, NULL, NULL);
+    p->opaque = gbe_program_new_from_source(p->source, p->build_log_max_sz, options, p->build_log, &p->build_log_sz);
     if (UNLIKELY(p->opaque == NULL)) {
-      err = CL_BUILD_PROGRAM_FAILURE;
+      if (p->build_log_sz > 0 && strstr(p->build_log, "error: error reading 'options'"))
+        err = CL_INVALID_BUILD_OPTIONS;
+      else
+        err = CL_BUILD_PROGRAM_FAILURE;
       goto error;
     }
 
diff --git a/src/cl_program.h b/src/cl_program.h
index 2cb547a..a6d75da 100644
--- a/src/cl_program.h
+++ b/src/cl_program.h
@@ -54,6 +54,9 @@ struct _cl_program {
   uint32_t source_type:2; /* Built from binary, source or LLVM */
   uint32_t is_built:1;    /* Did we call clBuildProgram on it? */
   char *build_opts;       /* The build options for this program */
+  size_t build_log_max_sz; /*build log maximum size in byte.*/
+  char *build_log;         /* The build log for this program. */
+  size_t build_log_sz;    /* The actual build log size.*/
 };
 
 /* Create a empty program */
-- 
1.7.9.5



More information about the Beignet mailing list