[Mesa-stable] [Mesa-dev] [PATCH] clover: Return CL_BUILD_ERROR for CL_PROGRAM_BUILD_STATUS when compilation fails

[Tom Stellard]

On Tue, Mar 24, 2015 at 10:36:19PM +0200, Francisco Jerez wrote:
> Jan Vesely <jan.vesely at rutgers.edu> writes:
> 
> > On Tue, 2015-03-24 at 22:24 +0200, Francisco Jerez wrote:
> >> Tom Stellard <thomas.stellard at amd.com> writes:
> >> 
> >> > Cc: 10.5 10.4 <mesa-stable at lists.freedesktop.org>
> >> > ---
> >> >  src/gallium/state_trackers/clover/core/program.cpp | 6 +++++-
> >> >  src/gallium/state_trackers/clover/core/program.hpp | 1 +
> >> >  2 files changed, 6 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/src/gallium/state_trackers/clover/core/program.cpp b/src/gallium/state_trackers/clover/core/program.cpp
> >> > index 8553ca7..06a6d92 100644
> >> > --- a/src/gallium/state_trackers/clover/core/program.cpp
> >> > +++ b/src/gallium/state_trackers/clover/core/program.cpp
> >> > @@ -49,6 +49,7 @@ program::build(const ref_vector<device> &devs, const char *opts,
> >> >           _binaries.erase(&dev);
> >> >           _logs.erase(&dev);
> >> >           _opts.erase(&dev);
> >> > +         _errs.erase(&dev);
> >> >  
> >> >           _opts.insert({ &dev, opts });
> >> >  
> >> > @@ -65,6 +66,7 @@ program::build(const ref_vector<device> &devs, const char *opts,
> >> >              _logs.insert({ &dev, log });
> >> >           } catch (const build_error &) {
> >> >              _logs.insert({ &dev, log });
> >> > +            _errs.insert(&dev);
> >> >              throw;
> >> >           }
> >> >        }
> >> > @@ -88,7 +90,9 @@ program::binary(const device &dev) const {
> >> >  
> >> >  cl_build_status
> >> >  program::build_status(const device &dev) const {
> >> > -   if (_binaries.count(&dev))
> >> > +   if (_errs.count(&dev))
> >> > +      return CL_BUILD_ERROR;
> >> > +   else if (_binaries.count(&dev))
> >> >        return CL_BUILD_SUCCESS;
> >> >     else
> >> >        return CL_BUILD_NONE;
> >> 
> >> Any reason you couldn't do something like:
> >> 
> >> |   if (_binaries.count(&dev))
> >> |      return CL_BUILD_SUCCESS;
> >> |   else if (_log.count(&dev))
> >> |      return CL_BUILD_ERROR;
> >> |   else
> >> |      return CL_BUILD_NONE;
> >> 
> >
> > Wouldn't this return error even if only warnings are produced?
> >
> No, warnings alone wouldn't cause _binaries.count(&dev) to be zero.
> 

At first I was thinking this wouldn't work, because we might get an
empty log message, but I realize now that this doesn't matter, because
dev will still be a key in the map even if log is empty.
I will fix this.

-Tom

> >> 
> >> > diff --git a/src/gallium/state_trackers/clover/core/program.hpp b/src/gallium/state_trackers/clover/core/program.hpp
> >> > index 661fa03..8c90199 100644
> >> > --- a/src/gallium/state_trackers/clover/core/program.hpp
> >> > +++ b/src/gallium/state_trackers/clover/core/program.hpp
> >> > @@ -73,6 +73,7 @@ namespace clover {
> >> >        std::map<const device *, module> _binaries;
> >> >        std::map<const device *, std::string> _logs;
> >> >        std::map<const device *, std::string> _opts;
> >> > +      std::set<const device *> _errs;
> >> >        std::string _source;
> >> >        ref_counter _kernel_ref_counter;
> >> >     };
> >> > -- 
> >> > 2.0.4
> >> _______________________________________________
> >> mesa-dev mailing list
> >> mesa-dev at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> > -- 
> > Jan Vesely <jan.vesely at rutgers.edu>




> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev