On Wed, Nov 27, 2013 at 06:44:04AM +0000, Yang, Rong R wrote:
>
>
> -----Original Message-----
> From: beignet-boun...@lists.freedesktop.org
> [mailto:beignet-boun...@lists.freedesktop.org] On Behalf Of Zhigang Gong
> Sent: Wednesday, November 27, 2013 10:47 AM
> To: beignet@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.g...@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet
> _______________________________________________
> Beignet mailing list
> Beignet@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet
_______________________________________________
Beignet mailing list
Beignet@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/beignet
