[Mesa-dev] [PATCH v4 045/129] nir: Add a concept of per-member structs and a lowering pass

[Kenneth Graunke]

On Thursday, May 31, 2018 10:02:28 PM PDT Jason Ekstrand wrote:
> This adds a concept of "members" to a variable with an interface type.
> It allows you to specify the full variable data for each member of the
> interface instead of once for the variable.  We also add a lowering pass
> to lower those variables to a sequence of variables and rewrite all the
> derefs accordingly.
> ---
>  src/compiler/Makefile.sources                   |   1 +
>  src/compiler/nir/meson.build                    |   1 +
>  src/compiler/nir/nir.h                          |  12 +
>  src/compiler/nir/nir_clone.c                    |   8 +
>  src/compiler/nir/nir_serialize.c                |  12 +
>  src/compiler/nir/nir_split_per_member_structs.c | 290 ++++++++++++++++++++++++
>  src/compiler/nir/nir_validate.c                 |   7 +
>  7 files changed, 331 insertions(+)
>  create mode 100644 src/compiler/nir/nir_split_per_member_structs.c

Jason and I discussed this on #dri-devel some today...a few things:

- I was concerned that this won't handle fully general structure/array
  nesting, with different decorations on various subfields.  It seems
  impossible to handle that without wholly rewriting how we represent
  structures in the IR.

- We used to do structure splitting right away, but we can't do that
  properly in SPIR-V because "pointers" may not terminate in a variable,
  and we need to be able to handle things part-way through a deref
  chain.

- We think that SPIR-V probably disallows cases that can't be handled
  by this code.  Hopefully. :/  Either way, those cases were probably
  already broken.

I don't like this at all but I don't want to tackle a struct rewrite
today, and this is probably the best we're going to get for now. :/

[snip]
> +static void
> +split_variable(struct nir_variable *var, nir_shader *shader,
> +               struct hash_table *var_to_member_map, void *dead_ctx)
> +{
> +   assert(var->state_slots == NULL);
> +
> +   /* Constant initializers are currently not handled */
> +   assert(var->constant_initializer == NULL);

Jason, could you please change this comment to:

   /* This pass should be run after lowering constant initializers. */

(I was concerned that the original code this replaced handled
initializers, and this code doesn't...but it's timing...we lower
them before this pass.  In the old code, we were splitting before
lowering, so we had to bother handling them.)

With that, patches 1-49 are
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180612/0d18bc44/attachment.sig>