[Mesa-dev] [PATCH 13/26] nir/lower_system_values: Add support for several computed values
Rob Clark
robdclark at gmail.com
Sat Mar 26 15:48:29 UTC 2016
On Fri, Mar 25, 2016 at 7:12 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> ---
> src/compiler/nir/nir.h | 3 ++
> src/compiler/nir/nir_lower_system_values.c | 74 ++++++++++++++++++++++++++++--
> 2 files changed, 74 insertions(+), 3 deletions(-)
>
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index 907dc34..d46858e 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -1678,6 +1678,9 @@ typedef struct nir_shader_compiler_options {
> * are simulated by floats.)
> */
> bool native_integers;
> +
> + /* Indicates that the driver only has zero-based vertex id */
> + bool vertex_id_zero_based;
btw, mind setting that to true in ir3_nir.c or reminding me when you
push this so that I can? (From quick look, I think it should be false
for vc4..)
> } nir_shader_compiler_options;
>
> typedef struct nir_shader_info {
> diff --git a/src/compiler/nir/nir_lower_system_values.c b/src/compiler/nir/nir_lower_system_values.c
> index 2bd787d..c1cd139 100644
> --- a/src/compiler/nir/nir_lower_system_values.c
> +++ b/src/compiler/nir/nir_lower_system_values.c
> @@ -55,9 +55,77 @@ convert_block(nir_block *block, void *void_state)
>
> b->cursor = nir_after_instr(&load_var->instr);
>
> - nir_intrinsic_op sysval_op =
> - nir_intrinsic_from_system_value(var->data.location);
> - nir_ssa_def *sysval = nir_load_system_value(b, sysval_op, 0);
> + nir_ssa_def *sysval;
> + switch (var->data.location) {
> + case SYSTEM_VALUE_GLOBAL_INVOCATION_ID: {
> + /* From the GLSL man page for gl_GlobalInvocationID:
> + *
> + * "The value of gl_GlobalInvocationID is equal to
> + * gl_WorkGroupID * gl_WorkGroupSize + gl_LocalInvocationID"
> + */
> +
> + nir_const_value local_size;
> + local_size.u32[0] = b->shader->info.cs.local_size[0];
> + local_size.u32[1] = b->shader->info.cs.local_size[1];
> + local_size.u32[2] = b->shader->info.cs.local_size[2];
> +
> + nir_ssa_def *group_id =
> + nir_load_system_value(b, nir_intrinsic_load_work_group_id, 0);
> + nir_ssa_def *local_id =
> + nir_load_system_value(b, nir_intrinsic_load_local_invocation_id, 0);
> +
> + sysval = nir_iadd(b, nir_imul(b, group_id,
> + nir_build_imm(b, 3, local_size)),
> + local_id);
> + break;
> + }
> +
> + case SYSTEM_VALUE_LOCAL_INVOCATION_INDEX: {
> + /* From the GLSL man page for gl_LocalInvocationIndex:
> + *
> + * ?The value of gl_LocalInvocationIndex is equal to
looks like that ? was supposed to be a "
> + * gl_LocalInvocationID.z * gl_WorkGroupSize.x *
> + * gl_WorkGroupSize.y + gl_LocalInvocationID.y *
> + * gl_WorkGroupSize.x + gl_LocalInvocationID.x"
> + */
> + nir_ssa_def *local_id =
> + nir_load_system_value(b, nir_intrinsic_load_local_invocation_id, 0);
> +
> + unsigned stride_y = b->shader->info.cs.local_size[0];
> + unsigned stride_z = b->shader->info.cs.local_size[0] *
> + b->shader->info.cs.local_size[1];
> +
> + sysval = nir_iadd(b, nir_imul(b, nir_channel(b, local_id, 2),
> + nir_imm_int(b, stride_z)),
> + nir_iadd(b, nir_imul(b, nir_channel(b, local_id, 1),
> + nir_imm_int(b, stride_y)),
> + nir_channel(b, local_id, 0)));
so this does get big enough to be difficult to read and the
stride_y/stride_z confused me ;-)
Not sure if something like this is an improvement:
nir_ssa_def *size_x = nir_imm_int(b, b->shader->info.cs.local_size[0]);
nir_ssa_def *size_y = nir_imm_int(b, b->shader->info.cs.local_size[1]);
/* gl_LocalInvocationID.z * gl_WorkGroupSize.x * gl_WorkGroupSize.y */
sysval = nir_imul(nir_imul(b, nir_channel(b, local_id, 2),
size_x), size_y);
/* + gl_LocalInvocationID.y * gl_WorkGroupSize.x */
sysval = nir_iadd(b, sysval, nir_imul(b, nir_channel(b,
local_id, 1), size_x));
/* + gl_LocalInvocationID.x */
sysval = nir_iadd(b, sysval, nir_channel(b, local_id, 0));
either way, I guess you don't need to pre-multiply size.x and size.y,
const prop pass will handle that, and not doing so makes it easier to
map back to the formula from the man page. And if you make them
nir_ssa_def's instead of unsigned then you could make the "math" part
of it less busy.
anyways, with those small comments,
Reviewed-by: Rob Clark <robdclark at gmail.com>
> + break;
> + }
> +
> + case SYSTEM_VALUE_VERTEX_ID:
> + if (b->shader->options->vertex_id_zero_based) {
> + sysval = nir_iadd(b,
> + nir_load_system_value(b, nir_intrinsic_load_vertex_id_zero_base, 0),
> + nir_load_system_value(b, nir_intrinsic_load_base_vertex, 0));
> + } else {
> + sysval = nir_load_system_value(b, nir_intrinsic_load_vertex_id, 0);
> + }
> + break;
> +
> + case SYSTEM_VALUE_INSTANCE_INDEX:
> + sysval = nir_iadd(b,
> + nir_load_system_value(b, nir_intrinsic_load_instance_id, 0),
> + nir_load_system_value(b, nir_intrinsic_load_base_instance, 0));
> + break;
> +
> + default: {
> + nir_intrinsic_op sysval_op =
> + nir_intrinsic_from_system_value(var->data.location);
> + sysval = nir_load_system_value(b, sysval_op, 0);
> + break;
> + } /* default */
> + }
>
> nir_ssa_def_rewrite_uses(&load_var->dest.ssa, nir_src_for_ssa(sysval));
> nir_instr_remove(&load_var->instr);
> --
> 2.5.0.400.gff86faf
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list