[Mesa-dev] [PATCH v5 07/21] clover/api: Rework the validation of devices for building

[Francisco Jerez]

Pierre Moreau <pierre.morrow at free.fr> writes:

> Signed-off-by: Pierre Moreau <pierre.morrow at free.fr>
> ---
>  src/gallium/state_trackers/clover/api/program.cpp  | 23 +++++++++++++---------
>  src/gallium/state_trackers/clover/core/program.cpp |  3 ++-
>  2 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/src/gallium/state_trackers/clover/api/program.cpp b/src/gallium/state_trackers/clover/api/program.cpp
> index 9d59668f8f..e97b6400fe 100644
> --- a/src/gallium/state_trackers/clover/api/program.cpp
> +++ b/src/gallium/state_trackers/clover/api/program.cpp
> @@ -32,6 +32,7 @@ namespace {
>     void
>     validate_build_common(const program &prog, cl_uint num_devs,
>                           const cl_device_id *d_devs,
> +                         ref_vector<device> &valid_devs,

I asked you to drop the valid_devs argument of this function the last
time around, because it should be equal to prog.devices() in all cases.
If it's not in some case (clLinkProgram?) we should fix the
corresponding program constructor to make sure that's the case.

Otherwise LGTM.

>                           void (*pfn_notify)(cl_program, void *),
>                           void *user_data) {
>        if (!pfn_notify && user_data)
> @@ -41,7 +42,7 @@ namespace {
>           throw error(CL_INVALID_OPERATION);
>  
>        if (any_of([&](const device &dev) {
> -               return !count(dev, prog.context().devices());
> +               return !count(dev, valid_devs);
>              }, objs<allow_empty_tag>(d_devs, num_devs)))
>           throw error(CL_INVALID_DEVICE);
>     }
> @@ -176,12 +177,13 @@ 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 valid_devs = ref_vector<device>(prog.devices());
> +   auto devs = (d_devs ? objs(d_devs, num_devs) : valid_devs);
>     const auto opts = std::string(p_opts ? p_opts : "") + " " +
>                       debug_get_option("CLOVER_EXTRA_BUILD_OPTIONS", "");
>  
> -   validate_build_common(prog, num_devs, d_devs, pfn_notify, user_data);
> +   validate_build_common(prog, num_devs, d_devs, valid_devs, pfn_notify,
> +                         user_data);
>  
>     if (prog.has_source) {
>        prog.compile(devs, opts);
> @@ -202,13 +204,14 @@ 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 valid_devs = ref_vector<device>(prog.devices());
> +   auto devs = (d_devs ? objs(d_devs, num_devs) : valid_devs);
>     const auto opts = std::string(p_opts ? p_opts : "") + " " +
>                       debug_get_option("CLOVER_EXTRA_COMPILE_OPTIONS", "");
>     header_map headers;
>  
> -   validate_build_common(prog, num_devs, d_devs, pfn_notify, user_data);
> +   validate_build_common(prog, num_devs, d_devs, valid_devs, pfn_notify,
> +                         user_data);
>  
>     if (bool(num_headers) != bool(header_names))
>        throw error(CL_INVALID_VALUE);
> @@ -280,11 +283,13 @@ clLinkProgram(cl_context d_ctx, cl_uint num_devs, const cl_device_id *d_devs,
>                       debug_get_option("CLOVER_EXTRA_LINK_OPTIONS", "");
>     auto progs = objs(d_progs, num_progs);
>     auto prog = create<program>(ctx);
> +   auto valid_devs = ref_vector<device>(ctx.devices());
>     auto devs = validate_link_devices(progs,
>                                       (d_devs ? objs(d_devs, num_devs) :
> -                                      ref_vector<device>(ctx.devices())));
> +                                      valid_devs));
>  
> -   validate_build_common(prog, num_devs, d_devs, pfn_notify, user_data);
> +   validate_build_common(prog, num_dwevs, d_devs, valid_devs, pfn_notify,
> +         user_data);
>  
>     try {
>        prog().link(devs, opts, progs);
> diff --git a/src/gallium/state_trackers/clover/core/program.cpp b/src/gallium/state_trackers/clover/core/program.cpp
> index ec71d99b01..62fa13efbf 100644
> --- a/src/gallium/state_trackers/clover/core/program.cpp
> +++ b/src/gallium/state_trackers/clover/core/program.cpp
> @@ -26,7 +26,8 @@
>  using namespace clover;
>  
>  program::program(clover::context &ctx, const std::string &source) :
> -   has_source(true), context(ctx), _source(source), _kernel_ref_counter(0) {
> +   has_source(true), context(ctx), _devices(ctx.devices()), _source(source),
> +   _kernel_ref_counter(0) {
>  }
>  
>  program::program(clover::context &ctx,
> -- 
> 2.16.3
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180325/87e8a043/attachment.sig>