[Beignet] [PATCH v2] Runtime: implement the get build log function and fix one build error check issue.
Yang, Rong R
rong.r.yang at intel.com
Wed Nov 27 00:49:04 PST 2013
LGTM, thanks.
-----Original Message-----
From: beignet-bounces at lists.freedesktop.org [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of Zhigang Gong
Sent: Wednesday, November 27, 2013 4:05 PM
To: beignet at lists.freedesktop.org
Cc: Gong, Zhigang
Subject: [Beignet] [PATCH v2] Runtime: implement the get build log function and fix one build error check issue.
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
_______________________________________________
Beignet mailing list
Beignet at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/beignet
More information about the Beignet
mailing list