[Beignet] [PATCH] Report build failures in backend to the build log
Yang, Rong R
rong.r.yang at intel.com
Wed Jul 13 08:43:15 UTC 2016
Sorry I missed this patch.
GBE has two type assert, one is internal debug assert, another one is caused by user's kernel.
I think the second assert is unreasonable and need to report the failures instead of assert.
Your patch is one of unreasonable assert, this patch is LGTM. And maybe we could clean all of the second type asserts in the future.
Pushed, thanks.
> -----Original Message-----
> From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of
> Rebecca N. Palmer
> Sent: Wednesday, July 13, 2016 3:36
> To: beignet at lists.freedesktop.org
> Subject: Re: [Beignet] [PATCH] Report build failures in backend to the build
> log
>
> I never got a reply to this; is there something wrong with it (i.e.
> should I stop including it in Debian's beignet) or are you just not interested in
> this feature?
>
> In the case I tested, the assert happens in LLVM 3.5 but not 3.7 (which
> instead returns the error "function with no prototype cannot use the
> spir_function calling convention"), but I don't know whether this applies to all
> missing-function errors.
>
> On 06/06/16 23:37, Rebecca N. Palmer wrote:
> > As noted at llvm_gen_backend:94, we currently lack a mechanism for
> > reporting failures in backend (beignet-managed) compiler passes to the
> > build log, and instead print the error to stderr and assert-fail.
> >
> > This patch creates such a mechanism, and uses it for "function not found".
> > Please note that it has not had much testing yet.
> >
> > Points for discussion/improvement:
> > -Does not currently print source location information.
> > -Are there other assertions that can be triggered by invalid input,
> > and hence should also use this?
> >
> > Signed-off-by: Rebecca Palmer <rebecca_palmer at zoho.com>
> >
> > diff --git a/backend/src/backend/program.cpp
> > b/backend/src/backend/program.cpp index 4f8167c..44cb60b 100644
> > --- a/backend/src/backend/program.cpp
> > +++ b/backend/src/backend/program.cpp
> > @@ -122,7 +122,7 @@ namespace gbe {
> > bool Program::buildFromLLVMFile(const char *fileName, const void*
> module, std::string &error, int optLevel) {
> > ir::Unit *unit = new ir::Unit();
> > llvm::Module * cloned_module = NULL;
> > - bool ret = true;
> > + bool ret = false;
> > if(module){
> > #if LLVM_VERSION_MAJOR == 3 && LLVM_VERSION_MINOR >= 8
> > cloned_module =
> > llvm::CloneModule((llvm::Module*)module).release();
> > @@ -133,7 +133,7 @@ namespace gbe {
> > bool strictMath = true;
> > if (fast_relaxed_math || !OCL_STRICT_CONFORMANCE)
> > strictMath = false;
> > - if (llvmToGen(*unit, fileName, module, optLevel, strictMath,
> OCL_PROFILING_LOG) == false) {
> > + if (llvmToGen(*unit, fileName, module, optLevel, strictMath,
> > + OCL_PROFILING_LOG, error) == false) {
> > if (fileName)
> > error = std::string(fileName) + " not found";
> > delete unit;
> > @@ -146,15 +146,19 @@ namespace gbe {
> > 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);
> > + 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);
> > + llvmToGen(*unit, fileName, module, 0, strictMath,
> > + OCL_PROFILING_LOG, error);
> > }
> > }
> > - assert(unit->getValid());
> > - if (!this->buildFromUnit(*unit, error))
> > - ret = false;
> > + if(unit->getValid()){
> > + std::string error2;
> > + if (this->buildFromUnit(*unit, error2)){
> > + ret = true;
> > + }
> > + error = error + error2;
> > + }
> > delete unit;
> > if(cloned_module){
> > delete (llvm::Module*) cloned_module; diff --git
> > a/backend/src/llvm/llvm_gen_backend.cpp
> > b/backend/src/llvm/llvm_gen_backend.cpp
> > index acad1b2..bb00aab 100644
> > --- a/backend/src/llvm/llvm_gen_backend.cpp
> > +++ b/backend/src/llvm/llvm_gen_backend.cpp
> > @@ -513,6 +513,7 @@ namespace gbe
> > Function *Func;
> > const Module *TheModule;
> > int btiBase;
> > + bool has_errors;
> > /*! legacyMode is for hardware before BDW,
> > * which do not support stateless memory access */
> > bool legacyMode;
> > @@ -528,6 +529,7 @@ namespace gbe
> > LI(0),
> > TheModule(0),
> > btiBase(BTI_RESERVED_NUM),
> > + has_errors(false),
> > legacyMode(true)
> > {
> > #if LLVM_VERSION_MAJOR == 3 && LLVM_VERSION_MINOR >=7 @@ -
> 2940,6
> > +2942,9 @@ namespace gbe
> > pass = PASS_EMIT_REGISTERS;
> > for (inst_iterator I = inst_begin(&F), E = inst_end(&F); I != E; ++I)
> > visit(*I);
> > +
> > + // Abort if this found an error (otherwise emitBasicBlock will assert)
> > + if(has_errors){return;}
> >
> > // First create all the labels (one per block) ...
> > for (Function::iterator BB = F.begin(), E = F.end(); BB != E;
> > ++BB) @@ -3749,11 +3754,8 @@ namespace gbe
> > break;
> > case GEN_OCL_NOT_FOUND:
> > default:
> > - std::cerr << "Caller instruction: " << std::endl;
> > - I.dump();
> > - std::cerr << "Callee function: " << std::endl;
> > - Callee->dump();
> > - GBE_ASSERT(0);
> > + has_errors = true;
> > + Func->getContext().emitError(&I,"function '" + fnName + "'
> > + not found or cannot be inlined");
> > };
> > }
> >
> > diff --git a/backend/src/llvm/llvm_to_gen.cpp
> > b/backend/src/llvm/llvm_to_gen.cpp
> > index 41723d1..02a69ec 100644
> > --- a/backend/src/llvm/llvm_to_gen.cpp
> > +++ b/backend/src/llvm/llvm_to_gen.cpp
> > @@ -26,6 +26,8 @@
> >
> > #include "llvm/llvm_gen_backend.hpp"
> > #include "llvm/llvm_to_gen.hpp"
> > +#include <llvm/IR/DiagnosticInfo.h>
> > +#include <llvm/IR/DiagnosticPrinter.h>
> > #include "sys/cvar.hpp"
> > #include "sys/platform.hpp"
> > #include "ir/unit.hpp"
> > @@ -249,8 +251,36 @@ namespace gbe
> > BVAR(OCL_OUTPUT_LLVM_AFTER_LINK, false);
> > BVAR(OCL_OUTPUT_LLVM_AFTER_GEN, false);
> >
> > + class gbeDiagnosticContext
> > + {
> > + public:
> > + gbeDiagnosticContext() : _str(""), messages(_str), printer(messages),
> _has_errors(false) {}
> > + void process(const llvm::DiagnosticInfo &diagnostic)
> > + {
> > + if (diagnostic.getSeverity() != DS_Remark) { // avoid noise from
> function inlining remarks
> > + diagnostic.print(printer);
> > + }
> > + if (diagnostic.getSeverity() == DS_Error) {
> > + _has_errors = true;
> > + }
> > + }
> > + std::string str(){return messages.str();}
> > + bool has_errors(){return _has_errors;}
> > + private:
> > + std::string _str;
> > + llvm::raw_string_ostream messages;
> > + llvm::DiagnosticPrinterRawOStream printer;
> > + bool _has_errors;
> > + };
> > +
> > + void gbeDiagnosticHandler(const llvm::DiagnosticInfo &diagnostic,
> > + void *context) {
> > + gbeDiagnosticContext *dc =
> reinterpret_cast<gbeDiagnosticContext*>(context);
> > + dc->process(diagnostic);
> > + }
> > +
> > bool llvmToGen(ir::Unit &unit, const char *fileName,const void* module,
> > - int optLevel, bool strictMath, int profiling)
> > + int optLevel, bool strictMath, int profiling,
> > + std::string &errors)
> > {
> > std::string errInfo;
> > std::unique_ptr<llvm::raw_fd_ostream> o = NULL; @@ -287,6 +317,9
> > @@ namespace gbe
> >
> > Module &mod = *M.get();
> > DataLayout DL(&mod);
> > +
> > + gbeDiagnosticContext dc;
> > + mod.getContext().setDiagnosticHandler(&gbeDiagnosticHandler,&dc);
> >
> > #if LLVM_VERSION_MAJOR == 3 && LLVM_VERSION_MINOR >= 7
> > mod.setDataLayout(DL);
> > @@ -345,6 +378,12 @@ namespace gbe
> > passes.add(createCFGOnlyPrinterPass());
> > passes.add(createGenPass(unit));
> > passes.run(mod);
> > + errors = dc.str();
> > + if(dc.has_errors()){
> > + unit.setValid(false);
> > + delete libraryInfo;
> > + return true;
> > + }
> >
> > // Print the code extra optimization passes
> > OUTPUT_BITCODE(AFTER_GEN, mod);
> > diff --git a/backend/src/llvm/llvm_to_gen.hpp
> > b/backend/src/llvm/llvm_to_gen.hpp
> > index 5667197..e0a6145 100644
> > --- a/backend/src/llvm/llvm_to_gen.hpp
> > +++ b/backend/src/llvm/llvm_to_gen.hpp
> > @@ -33,7 +33,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,
> > - int optLevel, bool strictMath, int profiling);
> > + int optLevel, bool strictMath, int profiling,
> > + std::string &errors);
> >
> > } /* namespace gbe */
> >
> >
>
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/beignet
More information about the Beignet
mailing list