[Mesa-dev] [PATCH v2 10/21] anv/nir: Use nir_variable's type if interface_type is null

[Neil Roberts]

Timothy Arceri <tarceri at itsqueeze.com> writes:

>
> glsl_get_sampler_dim() contains the following:
>
>     assert(glsl_type_is_sampler(type) || glsl_type_is_image(type));
>
> Which leads me to believe the code above should just be:
>
> const struct glsl_type *glsl_type = glsl_without_array(var->type);
>
> If you agree please squash this patch into the previous patch where you 
> can keep my r-b.

I agree, that sounds sensible.

It looks like var->interface_type was being overloaded as a place to
store the type for the image or sampler without the array type. The
comment above interface_type says:

 “For variables that are in an interface block or are an instance of an
  interface block, this is the GLSL_TYPE_INTERFACE type for that block.”

So it seems a little strange to use it for this purpose. It presumably
works because opaque types can’t be in interfaces so the interface_type
is otherwise unused. I guess with the previous patch interface_type will
always be NULL so you’re right that the check for whether it is NULL in
seems pointless.

Maybe we should however add a comment to the commit message for the
previous commit along these lines:

“The previous code appeared to be using var->interface_type as a place
to store the type of the variable without the enclosing array for images
and samplers. I guess this worked because opaque types can not appear in
interfaces so the interface_type is sort of unused. This patch removes
the overloading of var->interface_type and any places that needed the
type without the array can now just deduce it from var->type.”

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/20180607/c404d8cd/attachment.sig>