[Mesa-dev] [PATCH 1/3] spirv: Whack sampler/image pointers to uniform

[Lionel Landwerlin]

For the series :

Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>

On 11/01/2019 21:05, Jason Ekstrand wrote:
> A long time in a galaxy far far away, there was a GLSLang bug with how
> it handled samplers passed in as function parameters.  (The bug can be
> found here: https://github.com/KhronosGroup/glslang/issues/179.)
> Unfortunately, that version was shipped in several apps and has been
> causing heartburn for our SPIR-V parser ever since.
>
> Recent changes to NIR uncovered a moderately old bug in how we work
> around this issue.  In particular, we ended up with a deref_cast from
> uniform to local which is not a no-op cast so nir_opt_deref wasn't
> getting rid of the cast.  The only reason why it worked before was
> because someone just happened to call nir_fixup_deref_modes which
> "fixed" the cast (that shouldn't be happening) and then a later round of
> copy-prop would get rid of it.  The fact that the deref_cast survived
> that long without causing trouble for other parts of NIR is a bit
> surprising.
>
> Just whacking the mode of the pointer seems to fix it fairly
> unobtrusively.  Currently, only apps with this bug will have a local
> variable containing an image or sampler.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109304
> ---
>   src/compiler/spirv/vtn_variables.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/src/compiler/spirv/vtn_variables.c b/src/compiler/spirv/vtn_variables.c
> index 50cd1b42c69..6036295e61c 100644
> --- a/src/compiler/spirv/vtn_variables.c
> +++ b/src/compiler/spirv/vtn_variables.c
> @@ -1800,6 +1800,18 @@ vtn_pointer_from_ssa(struct vtn_builder *b, nir_ssa_def *ssa,
>      ptr->type = ptr_type->deref;
>      ptr->ptr_type = ptr_type;
>   
> +   /* To work around https://github.com/KhronosGroup/glslang/issues/179 we
> +    * need to whack the mode because it creates a function parameter with the
> +    * Function storage class even though it's a pointer to a sampler.  If we
> +    * don't do this, then NIR won't get rid of the deref_cast for us.
> +    */
> +   if (ptr->mode == vtn_variable_mode_function &&
> +       (ptr->type->base_type == vtn_base_type_sampler ||
> +        ptr->type->base_type == vtn_base_type_sampled_image)) {
> +      ptr->mode = vtn_variable_mode_uniform;
> +      nir_mode = nir_var_uniform;
> +   }
> +
>      if (vtn_pointer_uses_ssa_offset(b, ptr)) {
>         /* This pointer type needs to have actual storage */
>         vtn_assert(ptr_type->type);