[Mesa-dev] [PATCH] mesa/st: add support for NIR as possible driver IR

[Eric Anholt]

Rob Clark <robdclark at gmail.com> writes:

> diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp b/src/mesa/state_tracker/st_glsl_to_nir.cpp
> new file mode 100644
> index 0000000..c15c537
> --- /dev/null
> +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp

> +static void
> +st_nir_assign_uniform_locations(struct gl_program *prog,
> +                                struct gl_shader_program *shader_program,
> +                                struct exec_list *uniform_list, unsigned *size)
> +{
> +   int max = 0;
> +   int shaderidx = 0;
> +
> +   nir_foreach_variable(uniform, uniform_list) {
> +      int loc;
> +
> +      if (uniform->type->is_sampler()) {
> +         unsigned val;
> +         bool found = shader_program->UniformHash->get(val, uniform->name);
> +         loc = shaderidx++;
> +         assert(found);
> +         /* this ensure that nir_lower_samplers looks at the correct
> +          * shader_program->UniformStorage[location]:
> +          */
> +         uniform->data.location = val;
> +      } else if (strncmp(uniform->name, "gl_", 3) == 0) {
> +         const gl_state_index *const stateTokens = (gl_state_index *)uniform->state_slots[0].tokens;
> +         /* This state reference has already been setup by ir_to_mesa, but we'll
> +          * get the same index back here.
> +          */
> +         loc = _mesa_add_state_reference(prog->Parameters, stateTokens);
> +      } else {
> +         loc = _mesa_lookup_parameter_index(prog->Parameters, uniform->name);
> +      }
> +
> +      /* is there a better way to do this?  If we have something like:
> +       *
> +       *    struct S {
> +       *           float f;
> +       *           vec4 v;
> +       *    };
> +       *    uniform S color;
> +       *
> +       * Then what we get in prog->Parameters looks like:
> +       *
> +       *    0: Name=color.f, Type=6, DataType=1406, Size=1
> +       *    1: Name=color.v, Type=6, DataType=8b52, Size=4
> +       *
> +       * So the name doesn't match up and _mesa_lookup_parameter_index()
> +       * fails.  In this case just find the first matching "color.*"..
> +       *
> +       * Note for arrays you could end up w/ color[n].f, for example.
> +       */
> +      if (loc < 0) {
> +         int namelen = strlen(uniform->name);
> +         for (unsigned i = 0; i < prog->Parameters->NumParameters; i++) {
> +            struct gl_program_parameter *p = &prog->Parameters->Parameters[i];
> +            if ((strncmp(p->Name, uniform->name, namelen) == 0) &&
> +                ((p->Name[namelen] == '.') || (p->Name[namelen] == '['))) {
> +               loc = i;
> +               break;
> +            }
> +         }
> +      }

Yep, this looks like what you're stuck with.  You could make a helper
with a name related to _mesa_lookup_parameter_index(), or just always
use the block instead of _mesa_lookup_parameter_index(), I think, but
meh.

> +      uniform->data.driver_location = loc;
> +
> +      /*
> +       * UBO's have their own address spaces, so don't count them towards the
> +       * number of global uniforms
> +       */
> +      if ((uniform->data.mode == nir_var_uniform || uniform->data.mode == nir_var_shader_storage) &&
> +          uniform->interface_type != NULL)
> +         continue;

Why are we setting the driver_location according to a Parameters[]
lookup for UBOs?  I would think would skip that step entirely.  i965
also seemed to skip the driver_location setup for them.

With this UBO question answered somehow, I think I'll just give this
patch an acked-by.  I've read through it for quite some time, but it's a
huge change and most of what I care about for review in it is "does a
driver converting to this new path not regress?"
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160420/f88ce4fa/attachment.sig>