[Mesa-dev] [PATCH v2 13/21] nir/linker: Add gl_nir_link_uniforms()

[Neil Roberts]

Timothy Arceri <tarceri at itsqueeze.com> writes:

>> +static struct type_tree_entry *
>> +build_type_tree_for_type(const struct glsl_type *type)
>> +{
>
> Do we really need this? As far as I can tell we walk the types here to 
> build a tree then in nir_link_uniform() we walk the tree. Why not just 
> walk the types directly in nir_link_uniform()?

The tree is needed as a temporary place to store the next_index and
detect whether a node in the type tree is encountered for the first time
or not. The nodes can be encountered multiple times because if there is
an array of structs or array of arrays then we process the subtree of
the array once for each element in the array.

In nir_link_uniform, it effectively _is_ directly walking the glsl_type
tree. However it simultaneously walks the type_tree tree as well for
this extra side-band information.

The GLSL linker handles this instead by using a hash table to store the
next index (see set_opaque_indices in link_uniforms.cpp). The key for
the table is the name of the uniform with all array indices removed. I
suppose we could try to do something similar for the nir linker but it
would have to use a different key because we don’t have the names.
However my gut feeling is that that wouldn’t be any more efficient then
using this side-band tree and the code to generate the hash table key
looks quite involved.

If you can think of a simpler way to handle this then of course I am
open to suggestions.

Thanks for looking at the patch.

Regards,
- Neil
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180606/36b41f4b/attachment.sig>