[Mesa-dev] [PATCH] clover: fix clBuildProgram Piglit regression

Tom Stellard tom at stellard.net
Sun Nov 2 18:04:16 PST 2014


On Sun, Nov 02, 2014 at 08:03:31PM +0200, Francisco Jerez wrote:
> EdB <edb+mesa at sigluy.net> writes:
> 
> > should trigger CL_INVALID_VALUE
> > if device_list is NULL and num_devices is greater than zero.
> >
> > introduced by e5468dfa523be2a7a0d04bb9efcf8ae780957563
> 
> Tom, can you just drop the the vector of devices parameter and validate
> the d_devs/num_devs arguments from validate_build_program_common() by
> calling objs<allow_empty_tag>, as I suggested when I gave my R-b for
> your commit.
> 

The reason I kept the vector of devices is because if the
device list is NULL, then the device list from the context
need to be used.  I didn't want to duplicate this logic in
validate_build_program_common(), so I added the allow_empty_tag
to the API functions instead.

I think EdB's fix is a better solution, what do you think?

-Tom

> Thanks.
> 
> > ---
> >  src/gallium/state_trackers/clover/api/program.cpp | 20 +++++++++++---------
> >  1 file changed, 11 insertions(+), 9 deletions(-)
> >
> > diff --git a/src/gallium/state_trackers/clover/api/program.cpp b/src/gallium/state_trackers/clover/api/program.cpp
> > index 64c4a43..dc89730 100644
> > --- a/src/gallium/state_trackers/clover/api/program.cpp
> > +++ b/src/gallium/state_trackers/clover/api/program.cpp
> > @@ -27,7 +27,7 @@ using namespace clover;
> >  
> >  namespace {
> >     void validate_build_program_common(const program &prog, cl_uint num_devs,
> > -                                      const ref_vector<device> &devs,
> > +                                      ref_vector<device> &devs,
> >                                        void (*pfn_notify)(cl_program, void *),
> >                                        void *user_data) {
> >  
> > @@ -37,10 +37,14 @@ namespace {
> >        if (prog.kernel_ref_count())
> >           throw error(CL_INVALID_OPERATION);
> >  
> > -      if (any_of([&](const device &dev) {
> > -               return !count(dev, prog.context().devices());
> > -            }, devs))
> > -         throw error(CL_INVALID_DEVICE);
> > +      if (!num_devs) {
> > +         devs = prog.context().devices();
> > +      } else {
> > +         if (any_of([&](const device &dev) {
> > +                  return !count(dev, prog.context().devices());
> > +               }, devs))
> > +            throw error(CL_INVALID_DEVICE);
> > +      }
> >     }
> >  }
> >  
> > @@ -173,8 +177,7 @@ clBuildProgram(cl_program d_prog, cl_uint num_devs,
> >                 void (*pfn_notify)(cl_program, void *),
> >                 void *user_data) try {
> >     auto &prog = obj(d_prog);
> > -   auto devs = (d_devs ? objs(d_devs, num_devs) :
> > -                ref_vector<device>(prog.context().devices()));
> > +   auto devs = objs<allow_empty_tag>(d_devs, num_devs);
> >     auto opts = (p_opts ? p_opts : "");
> >  
> >     validate_build_program_common(prog, num_devs, devs, pfn_notify, user_data);
> > @@ -195,8 +198,7 @@ clCompileProgram(cl_program d_prog, cl_uint num_devs,
> >                   void (*pfn_notify)(cl_program, void *),
> >                   void *user_data) try {
> >     auto &prog = obj(d_prog);
> > -   auto devs = (d_devs ? objs(d_devs, num_devs) :
> > -                ref_vector<device>(prog.context().devices()));
> > +   auto devs = objs<allow_empty_tag>(d_devs, num_devs);
> >     auto opts = (p_opts ? p_opts : "");
> >     header_map headers;
> >  
> > -- 
> > 1.9.3
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev




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



More information about the mesa-dev mailing list