Re: [Mesa-dev] [PATCH v10 06/20] clover/api: Rework the validation of devices for building

2019-01-20 Thread Pierre Moreau 
Thank you for the review.
Do you think you’ll have the opportunity to have a look at patches 13 and 16?
(Patch 15 is also missing a review, but I found some improvements to be made
there.)

Thanks,
Pierre

On 2019-01-18 — 15:52, Francisco Jerez wrote:
> Pierre Moreau  writes:
> 
> > Reviewed-by: Francisco Jerez 
> >
> > Changes since:
> > * v5:
> >   - Drop the `valid_devs` argument to `validate_build_common()`
> > (Francisco Jerez)
> >   - Change `clLinkProgram()` to initialise `prog`’s devices prior to
> > calling `validate_build_common()`.
> > * v2:
> >   - validate_build_common no longer returns a list of devices (Francisco
> > Jerez);
> >   - Dropped duplicate checks (Francisco Jerez).
> >
> > Signed-off-by: Pierre Moreau 
> 
> The current revision of this patch is still:
> 
> Reviewed-by: Francisco Jerez 
> 
> > ---
> >  .../state_trackers/clover/api/program.cpp  | 18 +-
> >  .../state_trackers/clover/core/program.cpp |  3 ++-
> >  2 files changed, 11 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/gallium/state_trackers/clover/api/program.cpp 
> > b/src/gallium/state_trackers/clover/api/program.cpp
> > index 9d59668f8f6..891a002f3d0 100644
> > --- a/src/gallium/state_trackers/clover/api/program.cpp
> > +++ b/src/gallium/state_trackers/clover/api/program.cpp
> > @@ -41,7 +41,7 @@ namespace {
> >   throw error(CL_INVALID_OPERATION);
> >  
> >if (any_of([&](const device ) {
> > -   return !count(dev, prog.context().devices());
> > +   return !count(dev, prog.devices());
> >  }, objs(d_devs, num_devs)))
> >   throw error(CL_INVALID_DEVICE);
> > }
> > @@ -176,8 +176,8 @@ clBuildProgram(cl_program d_prog, cl_uint num_devs,
> > void (*pfn_notify)(cl_program, void *),
> > void *user_data) try {
> > auto  = obj(d_prog);
> > -   auto devs = (d_devs ? objs(d_devs, num_devs) :
> > -ref_vector(prog.context().devices()));
> > +   auto devs =
> > +  (d_devs ? objs(d_devs, num_devs) : 
> > ref_vector(prog.devices()));
> > const auto opts = std::string(p_opts ? p_opts : "") + " " +
> >   debug_get_option("CLOVER_EXTRA_BUILD_OPTIONS", "");
> >  
> > @@ -202,8 +202,8 @@ clCompileProgram(cl_program d_prog, cl_uint num_devs,
> >   void (*pfn_notify)(cl_program, void *),
> >   void *user_data) try {
> > auto  = obj(d_prog);
> > -   auto devs = (d_devs ? objs(d_devs, num_devs) :
> > -ref_vector(prog.context().devices()));
> > +   auto devs =
> > +   (d_devs ? objs(d_devs, num_devs) : 
> > ref_vector(prog.devices()));
> > const auto opts = std::string(p_opts ? p_opts : "") + " " +
> >   debug_get_option("CLOVER_EXTRA_COMPILE_OPTIONS", "");
> > header_map headers;
> > @@ -279,10 +279,10 @@ clLinkProgram(cl_context d_ctx, cl_uint num_devs, 
> > const cl_device_id *d_devs,
> > const auto opts = std::string(p_opts ? p_opts : "") + " " +
> >   debug_get_option("CLOVER_EXTRA_LINK_OPTIONS", "");
> > auto progs = objs(d_progs, num_progs);
> > -   auto prog = create(ctx);
> > -   auto devs = validate_link_devices(progs,
> > - (d_devs ? objs(d_devs, num_devs) :
> > -  ref_vector(ctx.devices(;
> > +   auto all_devs =
> > +  (d_devs ? objs(d_devs, num_devs) : 
> > ref_vector(ctx.devices()));
> > +   auto prog = create(ctx, all_devs);
> > +   auto devs = validate_link_devices(progs, all_devs);
> >  
> > validate_build_common(prog, num_devs, d_devs, pfn_notify, user_data);
> >  
> > diff --git a/src/gallium/state_trackers/clover/core/program.cpp 
> > b/src/gallium/state_trackers/clover/core/program.cpp
> > index ec71d99b017..62fa13efbf9 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 , const std::string ) :
> > -   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 ,
> > -- 
> > 2.20.1





signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v10 06/20] clover/api: Rework the validation of devices for building

2019-01-18 Thread Francisco Jerez 
Pierre Moreau  writes:

> Reviewed-by: Francisco Jerez 
>
> Changes since:
> * v5:
>   - Drop the `valid_devs` argument to `validate_build_common()`
> (Francisco Jerez)
>   - Change `clLinkProgram()` to initialise `prog`’s devices prior to
> calling `validate_build_common()`.
> * v2:
>   - validate_build_common no longer returns a list of devices (Francisco
> Jerez);
>   - Dropped duplicate checks (Francisco Jerez).
>
> Signed-off-by: Pierre Moreau 

The current revision of this patch is still:

Reviewed-by: Francisco Jerez 

> ---
>  .../state_trackers/clover/api/program.cpp  | 18 +-
>  .../state_trackers/clover/core/program.cpp |  3 ++-
>  2 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/src/gallium/state_trackers/clover/api/program.cpp 
> b/src/gallium/state_trackers/clover/api/program.cpp
> index 9d59668f8f6..891a002f3d0 100644
> --- a/src/gallium/state_trackers/clover/api/program.cpp
> +++ b/src/gallium/state_trackers/clover/api/program.cpp
> @@ -41,7 +41,7 @@ namespace {
>   throw error(CL_INVALID_OPERATION);
>  
>if (any_of([&](const device ) {
> -   return !count(dev, prog.context().devices());
> +   return !count(dev, prog.devices());
>  }, objs(d_devs, num_devs)))
>   throw error(CL_INVALID_DEVICE);
> }
> @@ -176,8 +176,8 @@ clBuildProgram(cl_program d_prog, cl_uint num_devs,
> void (*pfn_notify)(cl_program, void *),
> void *user_data) try {
> auto  = obj(d_prog);
> -   auto devs = (d_devs ? objs(d_devs, num_devs) :
> -ref_vector(prog.context().devices()));
> +   auto devs =
> +  (d_devs ? objs(d_devs, num_devs) : ref_vector(prog.devices()));
> const auto opts = std::string(p_opts ? p_opts : "") + " " +
>   debug_get_option("CLOVER_EXTRA_BUILD_OPTIONS", "");
>  
> @@ -202,8 +202,8 @@ clCompileProgram(cl_program d_prog, cl_uint num_devs,
>   void (*pfn_notify)(cl_program, void *),
>   void *user_data) try {
> auto  = obj(d_prog);
> -   auto devs = (d_devs ? objs(d_devs, num_devs) :
> -ref_vector(prog.context().devices()));
> +   auto devs =
> +   (d_devs ? objs(d_devs, num_devs) : 
> ref_vector(prog.devices()));
> const auto opts = std::string(p_opts ? p_opts : "") + " " +
>   debug_get_option("CLOVER_EXTRA_COMPILE_OPTIONS", "");
> header_map headers;
> @@ -279,10 +279,10 @@ clLinkProgram(cl_context d_ctx, cl_uint num_devs, const 
> cl_device_id *d_devs,
> const auto opts = std::string(p_opts ? p_opts : "") + " " +
>   debug_get_option("CLOVER_EXTRA_LINK_OPTIONS", "");
> auto progs = objs(d_progs, num_progs);
> -   auto prog = create(ctx);
> -   auto devs = validate_link_devices(progs,
> - (d_devs ? objs(d_devs, num_devs) :
> -  ref_vector(ctx.devices(;
> +   auto all_devs =
> +  (d_devs ? objs(d_devs, num_devs) : ref_vector(ctx.devices()));
> +   auto prog = create(ctx, all_devs);
> +   auto devs = validate_link_devices(progs, all_devs);
>  
> validate_build_common(prog, num_devs, d_devs, pfn_notify, user_data);
>  
> diff --git a/src/gallium/state_trackers/clover/core/program.cpp 
> b/src/gallium/state_trackers/clover/core/program.cpp
> index ec71d99b017..62fa13efbf9 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 , const std::string ) :
> -   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 ,
> -- 
> 2.20.1


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v10 06/20] clover/api: Rework the validation of devices for building

2019-01-08 Thread Pierre Moreau 
Reviewed-by: Francisco Jerez 

Changes since:
* v5:
  - Drop the `valid_devs` argument to `validate_build_common()`
(Francisco Jerez)
  - Change `clLinkProgram()` to initialise `prog`’s devices prior to
calling `validate_build_common()`.
* v2:
  - validate_build_common no longer returns a list of devices (Francisco
Jerez);
  - Dropped duplicate checks (Francisco Jerez).

Signed-off-by: Pierre Moreau 
---
 .../state_trackers/clover/api/program.cpp  | 18 +-
 .../state_trackers/clover/core/program.cpp |  3 ++-
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/src/gallium/state_trackers/clover/api/program.cpp 
b/src/gallium/state_trackers/clover/api/program.cpp
index 9d59668f8f6..891a002f3d0 100644
--- a/src/gallium/state_trackers/clover/api/program.cpp
+++ b/src/gallium/state_trackers/clover/api/program.cpp
@@ -41,7 +41,7 @@ namespace {
  throw error(CL_INVALID_OPERATION);
 
   if (any_of([&](const device ) {
-   return !count(dev, prog.context().devices());
+   return !count(dev, prog.devices());
 }, objs(d_devs, num_devs)))
  throw error(CL_INVALID_DEVICE);
}
@@ -176,8 +176,8 @@ clBuildProgram(cl_program d_prog, cl_uint num_devs,
void (*pfn_notify)(cl_program, void *),
void *user_data) try {
auto  = obj(d_prog);
-   auto devs = (d_devs ? objs(d_devs, num_devs) :
-ref_vector(prog.context().devices()));
+   auto devs =
+  (d_devs ? objs(d_devs, num_devs) : ref_vector(prog.devices()));
const auto opts = std::string(p_opts ? p_opts : "") + " " +
  debug_get_option("CLOVER_EXTRA_BUILD_OPTIONS", "");
 
@@ -202,8 +202,8 @@ clCompileProgram(cl_program d_prog, cl_uint num_devs,
  void (*pfn_notify)(cl_program, void *),
  void *user_data) try {
auto  = obj(d_prog);
-   auto devs = (d_devs ? objs(d_devs, num_devs) :
-ref_vector(prog.context().devices()));
+   auto devs =
+   (d_devs ? objs(d_devs, num_devs) : ref_vector(prog.devices()));
const auto opts = std::string(p_opts ? p_opts : "") + " " +
  debug_get_option("CLOVER_EXTRA_COMPILE_OPTIONS", "");
header_map headers;
@@ -279,10 +279,10 @@ clLinkProgram(cl_context d_ctx, cl_uint num_devs, const 
cl_device_id *d_devs,
const auto opts = std::string(p_opts ? p_opts : "") + " " +
  debug_get_option("CLOVER_EXTRA_LINK_OPTIONS", "");
auto progs = objs(d_progs, num_progs);
-   auto prog = create(ctx);
-   auto devs = validate_link_devices(progs,
- (d_devs ? objs(d_devs, num_devs) :
-  ref_vector(ctx.devices(;
+   auto all_devs =
+  (d_devs ? objs(d_devs, num_devs) : ref_vector(ctx.devices()));
+   auto prog = create(ctx, all_devs);
+   auto devs = validate_link_devices(progs, all_devs);
 
validate_build_common(prog, num_devs, d_devs, pfn_notify, user_data);
 
diff --git a/src/gallium/state_trackers/clover/core/program.cpp 
b/src/gallium/state_trackers/clover/core/program.cpp
index ec71d99b017..62fa13efbf9 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 , const std::string ) :
-   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 ,
-- 
2.20.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev