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

[Francisco Jerez]

Tom Stellard <tom at stellard.net> writes:

> 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.

No, that's fine, as I said earlier, when you end up using the default
device list, that list doesn't need to be error-checked, so there's no
need to pass it to validate_build_program_common(), you can just pass
the NULL/0 device list arguments.

> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141103/81f61fed/attachment.sig>