[Mesa-dev] [PATCH 13/26] nir/lower_system_values: Add support for several computed values
Jason Ekstrand
jason at jlekstrand.net
Mon Apr 11 06:41:47 UTC 2016
On Sat, Mar 26, 2016 at 8:48 AM, Rob Clark <robdclark at gmail.com> wrote:
> 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..)
>
Done. I also threw setting the flag for i965 into this patch. I was going
to do that on its own, but in here seems reasonable.
> > } 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 "
>
Yup.
> > + * 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.
>
I like that much better. I updated the patch accordingly (but without the
extra comments).
> anyways, with those small comments,
>
> Reviewed-by: Rob Clark <robdclark at gmail.com>
>
Thanks!
> > + 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160410/45d7a18b/attachment-0001.html>
More information about the mesa-dev
mailing list