[Mesa-dev] [PATCH 16/17] glsl: Add link time checks for GLSL precision qualifiers

[Ian Romanick]

On 07/29/2015 07:01 AM, Samuel Iglesias Gonsalvez wrote:
> From: Iago Toral Quiroga <itoral at igalia.com>
> 
> Currently, we only consider precision qualifiers at compile-time. This patch
> adds precision information to ir_variable so we can also do link time checks.
> Specifically, from the GLSL ES3 spec, 4.5.3 Precision Qualifiers:
> 
> "The same uniform declared in different shaders that are linked together
>  must have the same precision qualification."
> 
> Notice that this patch will check the above also for GLSL ES globals that are
> not uniforms. This is not explicitly stated in the spec, but seems to be
> the only consistent choice since we can only have one definition of a global
> all its declarations should be identical, including precision qualifiers.

That's not right.  Global variables from different stages that are not
inputs/outputs or uniforms are distinct... they don't even have to be
the same type.  ES shaders only allow a single compliation unit per
stage, so we don't have to worry about inter-stage globals.

> These checks don't affect desktop GLSL shaders because we ignore precision
> information in this case (all variables have precision GLSL_PRECISION_NONE).
> 
> Fixes the following 5 dEQP tests:
> dEQP-GLES3.functional.shaders.linkage.uniform.struct.precision_conflict_1
> dEQP-GLES3.functional.shaders.linkage.uniform.struct.precision_conflict_2
> dEQP-GLES3.functional.shaders.linkage.uniform.struct.precision_conflict_3
> dEQP-GLES3.functional.shaders.linkage.uniform.struct.precision_conflict_4
> dEQP-GLES3.functional.shaders.linkage.uniform.block.precision_mismatch
> ---
>  src/glsl/linker.cpp | 34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index 12b7780..fd68f43 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -958,13 +958,22 @@ cross_validate_globals(struct gl_shader_program *prog,
>                    if (var->type->is_record() && existing->type->is_record()
>                        && existing->type->record_compare(var->type)) {
>                       existing->type = var->type;
> -                  } else {
> +                  } else if (strcmp(var->type->name, existing->type->name)) {
>                       linker_error(prog, "%s `%s' declared as type "
>                                    "`%s' and type `%s'\n",
>                                    mode_string(var),
>                                    var->name, var->type->name,
>                                    existing->type->name);
>                       return;
> +                  } else {
> +                     /* The global is declared with the same type name but the type
> +                      * declarations mismatch (e.g. the same struct type name, but
> +                      * the actual struct declarations mismatch).
> +                      */
> +                     linker_error(prog, "%s `%s' declared with mismatching definitions "
> +                                  "of type `%s'\n",
> +                                  mode_string(var), var->name, var->type->name);
> +                     return;
>                    }
>  	       }
>  	    }
> @@ -1121,6 +1130,29 @@ cross_validate_globals(struct gl_shader_program *prog,
>                              mode_string(var), var->name);
>                 return;
>              }
> +            /* From the GLSL ES3 spec, 4.5.3 Precision qualifiers:
> +             *
> +             * "The same uniform declared in different shaders that are linked
> +             *  together must have the same precision qualification."
> +             *
> +             * In the GLSL ES2 spec this was resolved in the issue amendments
> +             * (10.3 Precision Qualifiers). The GLSL ES1 spec overlooked this,
> +             * but seems like an obvious error since we can only have one
> +             * consistent definition of a global.
> +             *
> +             * The desktop GLSL spec does not include this reference
> +             * because precision qualifiers are ignored. We will never
> +             * hit this scenario in desktop GLSL though because we always set
> +             * the precision of variables to GLSL_PRECISION_NONE.
> +             */
> +            if (var->data.mode == ir_var_uniform) {
> +               if (existing->data.precision != var->data.precision) {
> +                  linker_error(prog, "declarations for %s `%s` have "
> +                               "mismatching precision qualifiers\n",
> +                               mode_string(var), var->name);
> +                  return;
> +               }
> +            }
>  	 } else
>  	    variables.add_variable(var);
>        }
>