[Mesa-dev] [PATCH v3 04/14] i965/anv: Add uniform for a CS thread local base ID

[Jason Ekstrand]

I don't think "anv" is the right prefix for this patch.  Really, it's
adding local_id_index to cs_prog_data.

By and large, I think the code at the end of the series is what we want.  I
made a few comments but not many.  However, I think we could make it a bit
more bisectable if we wanted.  If you don't want to go through the effort,
feel free to just land the series as is (with the couple of suggestions
taken into account) with

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>

If you want to be a pedant, I think we could do something like the
following:

 1) Patch 7 with the anv changes as well
 2) This patch without the anv bits and with local_id_index always
initialized to -1 by brw_compile_cs
 3) Add the NIR pass
 4) Patch 8 only with a hack to set cross_thread_supported to false always
 5) Hook things up in i965
 6) Hook thnigs up in anv
 7) A single patch that properly sets local_id_index, runs the NIR pass,
and sets cross_thread_supported based on gen.

That would make the 'flip the switch" patch very small, probably a
half-dozen lines.  If you're going to be a pedant don't be a pedant on your
holiday... :-)

On Sun, May 29, 2016 at 3:38 PM, Jordan Justen <jordan.l.justen at intel.com>
wrote:

> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> ---
>  src/intel/vulkan/anv_pipeline.c          | 4 ++++
>  src/mesa/drivers/dri/i965/brw_compiler.h | 1 +
>  src/mesa/drivers/dri/i965/brw_cs.c       | 3 +++
>  src/mesa/drivers/dri/i965/brw_fs.cpp     | 8 ++++++++
>  4 files changed, 16 insertions(+)
>
> diff --git a/src/intel/vulkan/anv_pipeline.c
> b/src/intel/vulkan/anv_pipeline.c
> index 789bc1a..504f0be 100644
> --- a/src/intel/vulkan/anv_pipeline.c
> +++ b/src/intel/vulkan/anv_pipeline.c
> @@ -338,6 +338,10 @@ anv_pipeline_compile(struct anv_pipeline *pipeline,
>        pipeline->needs_data_cache = true;
>     }
>
> +   if (stage == MESA_SHADER_COMPUTE)
> +      ((struct brw_cs_prog_data *)prog_data)->thread_local_id_index =
> +         prog_data->nr_params++; /* The CS Thread ID uniform */
> +
>     if (nir->info.num_ssbos > 0)
>        pipeline->needs_data_cache = true;
>
> diff --git a/src/mesa/drivers/dri/i965/brw_compiler.h
> b/src/mesa/drivers/dri/i965/brw_compiler.h
> index 0844694..bed969c 100644
> --- a/src/mesa/drivers/dri/i965/brw_compiler.h
> +++ b/src/mesa/drivers/dri/i965/brw_compiler.h
> @@ -433,6 +433,7 @@ struct brw_cs_prog_data {
>     bool uses_barrier;
>     bool uses_num_work_groups;
>     unsigned local_invocation_id_regs;
> +   int thread_local_id_index;
>
>     struct {
>        /** @{
> diff --git a/src/mesa/drivers/dri/i965/brw_cs.c
> b/src/mesa/drivers/dri/i965/brw_cs.c
> index a9cbde9..2a25584 100644
> --- a/src/mesa/drivers/dri/i965/brw_cs.c
> +++ b/src/mesa/drivers/dri/i965/brw_cs.c
> @@ -93,6 +93,9 @@ brw_codegen_cs_prog(struct brw_context *brw,
>      */
>     int param_count = cp->program.Base.nir->num_uniforms / 4;
>
> +   /* The backend also sometimes add a param for the thread local id. */
> +   prog_data.thread_local_id_index = param_count++;
> +
>     /* The backend also sometimes adds params for texture size. */
>     param_count += 2 *
> ctx->Const.Program[MESA_SHADER_COMPUTE].MaxTextureImageUnits;
>     prog_data.base.param =
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index f5add6e..f7753db 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -6469,6 +6469,14 @@ brw_compile_cs(const struct brw_compiler *compiler,
> void *log_data,
>                                        true);
>     brw_nir_lower_cs_shared(shader);
>     prog_data->base.total_shared += shader->num_shared;
> +
> +   /* Now that we cloned the nir_shader, we can update num_uniforms based
> on
> +    * the thread_local_id_index.
> +    */
> +   shader->num_uniforms =
> +      MAX2(shader->num_uniforms,
> +           (unsigned)4 * (prog_data->thread_local_id_index + 1));
> +
>     shader = brw_postprocess_nir(shader, compiler->devinfo, true);
>
>     prog_data->local_size[0] = shader->info.cs.local_size[0];
> --
> 2.8.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160530/e38e2114/attachment-0001.html>