[Beignet] [PATCH 2/2] Runtime: implement the get build log function and fix one build error check issue.
Zhigang Gong
zhigang.gong at linux.intel.com
Tue Nov 26 23:13:31 PST 2013
On Wed, Nov 27, 2013 at 06:44:04AM +0000, Yang, Rong R wrote:
>
>
> -----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 10:47 AM
> To: beignet at lists.freedesktop.org
> Cc: Gong, Zhigang
> Subject: [Beignet] [PATCH 2/2] 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.
>
> Signed-off-by: Zhigang Gong <zhigang.gong at intel.com>
> ---
> backend/src/backend/program.cpp | 53 ++++++++++++++++++++++++++-------------
> src/cl_api.c | 5 ++--
> src/cl_program.c | 13 +++++++---
> src/cl_program.h | 3 +++
> 4 files changed, 51 insertions(+), 23 deletions(-)
>
> diff --git a/backend/src/backend/program.cpp b/backend/src/backend/program.cpp index 093febc..24bf6cb 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,34 @@ 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);
> +
> + size_t errStrPos = ErrorString.copy(err, stringSize - 1, 0);
> + ErrorString.clear();
> + if (errSize)
> + *errSize = errStrPos;
> 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 (errStrPos < stringSize - 1 && ErrorString.size() > 0) {
> + size_t errLen;
> + errLen = ErrorString.copy(err + errStrPos, stringSize - errStrPos, 0);
> + if (errSize)
> + *errSize += errLen;
> + }
> OS.close();
> + return true;
> }
>
> extern std::string ocl_stdlib_str;
> @@ -640,15 +656,18 @@ 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();
> + p = gbe_program_new_from_llvm(llName.c_str(), stringSize, err, errSize);
> Do you need adjust stringSize and err here? And errSize return from gbe_program_new_from_llvm don't include buildModuleFromSource.
You are right. We should keep the build log for the buildModuleFromSource even the build complete successfully.
I will send a new version to fix that.
Thanks,
Zhigang Gong.
>
> + 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
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet
More information about the Beignet
mailing list