[Mesa-dev] [PATCH v5 5/7] glsl: Add precision information to ir_variable

Iago Toral itoral at igalia.com
Tue Nov 10 02:26:07 PST 2015


On Fri, 2015-11-06 at 14:03 +0200, Tapani Pälli wrote:
> From: Iago Toral Quiroga <itoral at igalia.com>
> 
> We will need this later on when we implement proper support for
> precision qualifiers in the drivers and also to do link time checks for
> uniforms as indicated by the spec.
> 
> This patch also adds compile-time checks for variables without precision
> information (currently, Mesa only checks that a default precision is set
> for floats in fragment shaders).
> 
> As indicated by Ian, the addition of the precision information to
> ir_variable has been done using a bitfield and pahole to identify an
> available hole so that memory requirements for ir_variable stay the
> same.
> 
> v2 (Ian):
>   - Avoid if-ladders by defining arrays of supported sampler names and
>     indexing
>     into them with type->sampler_array + 2 * type->sampler_shadow
>   - Make the code that selects the precision qualifier to use an utility
>     function
>   - Fix a typo
> 
> v3 (Tapani):
>   - rebased
>   - squashed in "Precision qualifiers are not allowed on structs"
>   - fixed select_gles_precision for sampler arrays
>   - fixed precision_qualifier_allowed for arrays of structs
> 
> v4 (Tapani):
>   - add atomic_uint handling
>   - do not allow precision qualifier on images
>   (issues reported by Marta)
> 
> v5 (Tapani):
>   - support precision qualifier on image types
> ---
>  src/glsl/ast_to_hir.cpp     | 296 ++++++++++++++++++++++++++++++++++++++++----
>  src/glsl/ir.h               |  13 ++
>  src/glsl/nir/glsl_types.cpp |   4 +
>  src/glsl/nir/glsl_types.h   |  11 ++
>  4 files changed, 301 insertions(+), 23 deletions(-)
> 
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index b6d662b..1240615 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -2189,10 +2189,10 @@ precision_qualifier_allowed(const glsl_type *type)
>      * From this, we infer that GLSL 1.30 (and later) should allow precision
>      * qualifiers on sampler types just like float and integer types.
>      */
> -   return type->is_float()
> +   return (type->is_float()
>         || type->is_integer()
> -       || type->is_record()
> -       || type->contains_opaque();
> +       || type->contains_opaque())
> +       && !type->without_array()->is_record();
>  }
>  
>  const glsl_type *
> @@ -2210,31 +2210,268 @@ ast_type_specifier::glsl_type(const char **name,
>     return type;
>  }
>  
> -const glsl_type *
> -ast_fully_specified_type::glsl_type(const char **name,
> -                                    struct _mesa_glsl_parse_state *state) const
> +/**
> + * From the OpenGL ES 3.0 spec, 4.5.4 Default Precision Qualifiers:
> + *
> + * "The precision statement
> + *
> + *    precision precision-qualifier type;
> + *
> + *  can be used to establish a default precision qualifier. The type field can
> + *  be either int or float or any of the sampler types, (...) If type is float,
> + *  the directive applies to non-precision-qualified floating point type
> + *  (scalar, vector, and matrix) declarations. If type is int, the directive
> + *  applies to all non-precision-qualified integer type (scalar, vector, signed,
> + *  and unsigned) declarations."
> + *
> + * We use the symbol table to keep the values of the default precisions for
> + * each 'type' in each scope and we use the 'type' string from the precision
> + * statement as key in the symbol table. When we want to retrieve the default
> + * precision associated with a given glsl_type we need to know the type string
> + * associated with it. This is what this function returns.
> + */
> +static const char *
> +get_type_name_for_precision_qualifier(const glsl_type *type)
>  {
> -   const struct glsl_type *type = this->specifier->glsl_type(name, state);
> -
> -   if (type == NULL)
> -      return NULL;
> +   switch (type->base_type) {
> +   case GLSL_TYPE_FLOAT:
> +      return "float";
> +   case GLSL_TYPE_UINT:
> +   case GLSL_TYPE_INT:
> +      return "int";
> +   case GLSL_TYPE_ATOMIC_UINT:
> +      return "atomic_uint";
> +   case GLSL_TYPE_IMAGE:
> +   /* fallthrough */

I think this is not correct. As far as I understand the spec, we can set
a default precision for any of the image types:

image2D
image3D
imageCube
image2DArray
iimage2D
iimage3D
iimageCube
iimage2DArray
uimage2D
uimage3D
uimageCube
uimage2DArray

but here you are re-using the precisions from samplers, so if we do
this:

#precision lowp sampler2D;
#precision highp image2D;

the latter statement is ignored, and the former affects both samplers
and images, which is not what we want.

Iago

> +   case GLSL_TYPE_SAMPLER: {
> +      const unsigned type_idx =
> +         type->sampler_array + 2 * type->sampler_shadow;
> +      const unsigned offset = type->base_type == GLSL_TYPE_SAMPLER ? 0 : 4;
> +      assert(type_idx < 4);
> +      switch (type->sampler_type) {
> +      case GLSL_TYPE_FLOAT:
> +         switch (type->sampler_dimensionality) {
> +         case GLSL_SAMPLER_DIM_1D: {
> +            assert(type->base_type == GLSL_TYPE_SAMPLER);
> +            static const char *const names[4] = {
> +              "sampler1D", "sampler1DArray",
> +              "sampler1DShadow", "sampler1DArrayShadow"
> +            };
> +            return names[type_idx];
> +         }
> +         case GLSL_SAMPLER_DIM_2D: {
> +            static const char *const names[8] = {
> +              "sampler2D", "sampler2DArray",
> +              "sampler2DShadow", "sampler2DArrayShadow",
> +              "image2D", "image2DArray", NULL, NULL
> +            };
> +            return names[offset + type_idx];
> +         }
> +         case GLSL_SAMPLER_DIM_3D: {
> +            static const char *const names[8] = {
> +              "sampler3D", NULL, NULL, NULL,
> +              "image3D", NULL, NULL, NULL
> +            };
> +            return names[offset + type_idx];
> +         }
> +         case GLSL_SAMPLER_DIM_CUBE: {
> +            static const char *const names[8] = {
> +              "samplerCube", "samplerCubeArray",
> +              "samplerCubeShadow", "samplerCubeArrayShadow",
> +              "imageCube", NULL, NULL, NULL
> +            };
> +            return names[offset + type_idx];
> +         }
> +         case GLSL_SAMPLER_DIM_MS: {
> +            assert(type->base_type == GLSL_TYPE_SAMPLER);
> +            static const char *const names[4] = {
> +              "sampler2DMS", "sampler2DMSArray", NULL, NULL
> +            };
> +            return names[type_idx];
> +         }
> +         case GLSL_SAMPLER_DIM_RECT: {
> +            assert(type->base_type == GLSL_TYPE_SAMPLER);
> +            static const char *const names[4] = {
> +              "samplerRect", NULL, "samplerRectShadow", NULL
> +            };
> +            return names[type_idx];
> +         }
> +         case GLSL_SAMPLER_DIM_BUF: {
> +            assert(type->base_type == GLSL_TYPE_SAMPLER);
> +            static const char *const names[4] = {
> +              "samplerBuffer", NULL, NULL, NULL
> +            };
> +            return names[type_idx];
> +         }
> +         case GLSL_SAMPLER_DIM_EXTERNAL: {
> +            assert(type->base_type == GLSL_TYPE_SAMPLER);
> +            static const char *const names[4] = {
> +              "samplerExternalOES", NULL, NULL, NULL
> +            };
> +            return names[type_idx];
> +         }
> +         default:
> +            unreachable("Unsupported sampler/image dimensionality");
> +         } /* sampler/image float dimensionality */
> +         break;
> +      case GLSL_TYPE_INT:
> +         switch (type->sampler_dimensionality) {
> +         case GLSL_SAMPLER_DIM_1D: {
> +            assert(type->base_type == GLSL_TYPE_SAMPLER);
> +            static const char *const names[4] = {
> +              "isampler1D", "isampler1DArray", NULL, NULL
> +            };
> +            return names[type_idx];
> +         }
> +         case GLSL_SAMPLER_DIM_2D: {
> +            static const char *const names[8] = {
> +              "isampler2D", "isampler2DArray", NULL, NULL,
> +              "iimage2D", "iimage2DArray", NULL, NULL
> +            };
> +            return names[offset + type_idx];
> +         }
> +         case GLSL_SAMPLER_DIM_3D: {
> +            static const char *const names[8] = {
> +              "isampler3D", NULL, NULL, NULL,
> +              "iimage3D", NULL, NULL, NULL
> +            };
> +            return names[offset + type_idx];
> +         }
> +         case GLSL_SAMPLER_DIM_CUBE: {
> +            static const char *const names[8] = {
> +              "isamplerCube", "isamplerCubeArray", NULL, NULL,
> +              "iimageCube", NULL, NULL, NULL
> +            };
> +            return names[offset + type_idx];
> +         }
> +         case GLSL_SAMPLER_DIM_MS: {
> +            assert(type->base_type == GLSL_TYPE_SAMPLER);
> +            static const char *const names[4] = {
> +              "isampler2DMS", "isampler2DMSArray", NULL, NULL
> +            };
> +            return names[type_idx];
> +         }
> +         case GLSL_SAMPLER_DIM_RECT: {
> +            assert(type->base_type == GLSL_TYPE_SAMPLER);
> +            static const char *const names[4] = {
> +              "isamplerRect", NULL, "isamplerRectShadow", NULL
> +            };
> +            return names[type_idx];
> +         }
> +         case GLSL_SAMPLER_DIM_BUF: {
> +            assert(type->base_type == GLSL_TYPE_SAMPLER);
> +            static const char *const names[4] = {
> +              "isamplerBuffer", NULL, NULL, NULL
> +            };
> +            return names[type_idx];
> +         }
> +         default:
> +            unreachable("Unsupported isampler/iimage dimensionality");
> +         } /* sampler/image int dimensionality */
> +         break;
> +      case GLSL_TYPE_UINT:
> +         switch (type->sampler_dimensionality) {
> +         case GLSL_SAMPLER_DIM_1D: {
> +            assert(type->base_type == GLSL_TYPE_SAMPLER);
> +            static const char *const names[4] = {
> +              "usampler1D", "usampler1DArray", NULL, NULL
> +            };
> +            return names[type_idx];
> +         }
> +         case GLSL_SAMPLER_DIM_2D: {
> +            static const char *const names[8] = {
> +              "usampler2D", "usampler2DArray", NULL, NULL,
> +              "uimage2D", "uimage2DArray", NULL, NULL,
> +            };
> +            return names[offset + type_idx];
> +         }
> +         case GLSL_SAMPLER_DIM_3D: {
> +            static const char *const names[8] = {
> +              "usampler3D", NULL, NULL, NULL,
> +              "uimage3D", NULL, NULL, NULL,
> +            };
> +            return names[offset + type_idx];
> +         }
> +         case GLSL_SAMPLER_DIM_CUBE: {
> +            static const char *const names[8] = {
> +              "usamplerCube", "usamplerCubeArray", NULL, NULL,
> +              "uimageCube", NULL, NULL, NULL
> +            };
> +            return names[offset + type_idx];
> +         }
> +         case GLSL_SAMPLER_DIM_MS: {
> +            assert(type->base_type == GLSL_TYPE_SAMPLER);
> +            static const char *const names[4] = {
> +              "usampler2DMS", "usampler2DMSArray", NULL, NULL
> +            };
> +            return names[type_idx];
> +         }
> +         case GLSL_SAMPLER_DIM_RECT: {
> +            assert(type->base_type == GLSL_TYPE_SAMPLER);
> +            static const char *const names[4] = {
> +              "usamplerRect", NULL, "usamplerRectShadow", NULL
> +            };
> +            return names[type_idx];
> +         }
> +         case GLSL_SAMPLER_DIM_BUF: {
> +            assert(type->base_type == GLSL_TYPE_SAMPLER);
> +            static const char *const names[4] = {
> +              "usamplerBuffer", NULL, NULL, NULL
> +            };
> +            return names[type_idx];
> +         }
> +         default:
> +            unreachable("Unsupported usampler/uimage dimensionality");
> +         } /* sampler/image uint dimensionality */
> +         break;
> +      default:
> +         unreachable("Unsupported sampler/image type");
> +      } /* sampler/image type */
> +      break;
> +   } /* GLSL_TYPE_SAMPLER/GLSL_TYPE_IMAGE */
> +   break;
> +   default:
> +      unreachable("Unsupported type");
> +   } /* base type */
> +}
>  
> -   /* The fragment language does not define a default precision value
> -    * for float types, so check that one is defined if the type declaration
> -    * isn't providing one explictly.
> +static unsigned
> +select_gles_precision(unsigned qual_precision,
> +                      const glsl_type *type,
> +                      struct _mesa_glsl_parse_state *state, YYLTYPE *loc)
> +{
> +   /* Precision qualifiers do not have any meaning in Desktop GLSL.
> +    * In GLES we take the precision from the type qualifier if present,
> +    * otherwise, if the type of the variable allows precision qualifiers at
> +    * all, we look for the default precision qualifier for that type in the
> +    * current scope.
>      */
> -   if (type->base_type == GLSL_TYPE_FLOAT
> -       && state->es_shader
> -       && state->stage == MESA_SHADER_FRAGMENT
> -       && this->qualifier.precision == ast_precision_none
> -       && state->symbols->get_default_precision_qualifier("float") == ast_precision_none) {
> -      YYLTYPE loc = this->get_location();
> -      _mesa_glsl_error(&loc, state,
> -                       "no precision specified this scope for type `%s'",
> -                       type->name);
> +   assert(state->es_shader);
> +
> +   unsigned precision = GLSL_PRECISION_NONE;
> +   if (qual_precision) {
> +      precision = qual_precision;
> +   } else if (precision_qualifier_allowed(type)) {
> +      const char *type_name =
> +         get_type_name_for_precision_qualifier(type->without_array());
> +      assert(type_name != NULL);
> +
> +      precision =
> +         state->symbols->get_default_precision_qualifier(type_name);
> +      if (precision == ast_precision_none) {
> +         _mesa_glsl_error(loc, state,
> +                          "No precision specified in this scope for type `%s'",
> +                          type->name);
> +      }
>     }
> +   return precision;
> +}
>  
> -   return type;
> +const glsl_type *
> +ast_fully_specified_type::glsl_type(const char **name,
> +                                    struct _mesa_glsl_parse_state *state) const
> +{
> +   return this->specifier->glsl_type(name, state);
>  }
>  
>  /**
> @@ -2772,6 +3009,12 @@ apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual,
>     if (qual->flags.q.sample)
>        var->data.sample = 1;
>  
> +   /* Precision qualifiers do not hold any meaning in Desktop GLSL */
> +   if (state->es_shader) {
> +      var->data.precision =
> +         select_gles_precision(qual->precision, var->type, state, loc);
> +   }
> +
>     if (state->stage == MESA_SHADER_GEOMETRY &&
>         qual->flags.q.out && qual->flags.q.stream) {
>        var->data.stream = qual->stream;
> @@ -6635,6 +6878,13 @@ ast_interface_block::hir(exec_list *instructions,
>           if (var_mode == ir_var_shader_in || var_mode == ir_var_uniform)
>              var->data.read_only = true;
>  
> +         /* Precision qualifiers do not have any meaning in Desktop GLSL */
> +         if (state->es_shader) {
> +            var->data.precision =
> +               select_gles_precision(fields[i].precision, fields[i].type,
> +                                     state, &loc);
> +         }
> +
>           if (fields[i].matrix_layout == GLSL_MATRIX_LAYOUT_INHERITED) {
>              var->data.matrix_layout = matrix_layout == GLSL_MATRIX_LAYOUT_INHERITED
>                 ? GLSL_MATRIX_LAYOUT_COLUMN_MAJOR : matrix_layout;
> diff --git a/src/glsl/ir.h b/src/glsl/ir.h
> index 9c9f22d..b90451c 100644
> --- a/src/glsl/ir.h
> +++ b/src/glsl/ir.h
> @@ -770,6 +770,19 @@ public:
>        unsigned index:1;
>  
>        /**
> +       * Precision qualifier.
> +       *
> +       * In desktop GLSL we do not care about precision qualifiers at all, in
> +       * fact, the spec says that precision qualifiers are ignored.
> +       *
> +       * To make things easy, we make it so that this field is always
> +       * GLSL_PRECISION_NONE on desktop shaders. This way all the variables
> +       * have the same precision value and the checks we add in the compiler
> +       * for this field will never break a desktop shader compile.
> +       */
> +      unsigned precision:2;
> +
> +      /**
>         * \brief Layout qualifier for gl_FragDepth.
>         *
>         * This is not equal to \c ir_depth_layout_none if and only if this
> diff --git a/src/glsl/nir/glsl_types.cpp b/src/glsl/nir/glsl_types.cpp
> index 1c66dce..975b815 100644
> --- a/src/glsl/nir/glsl_types.cpp
> +++ b/src/glsl/nir/glsl_types.cpp
> @@ -162,6 +162,7 @@ glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields,
>        this->fields.structure[i].sample = fields[i].sample;
>        this->fields.structure[i].matrix_layout = fields[i].matrix_layout;
>        this->fields.structure[i].patch = fields[i].patch;
> +      this->fields.structure[i].precision = fields[i].precision;
>     }
>  
>     mtx_unlock(&glsl_type::mutex);
> @@ -779,6 +780,9 @@ glsl_type::record_compare(const glsl_type *b) const
>        if (this->fields.structure[i].image_restrict
>            != b->fields.structure[i].image_restrict)
>           return false;
> +      if (this->fields.structure[i].precision
> +          != b->fields.structure[i].precision)
> +         return false;
>     }
>  
>     return true;
> diff --git a/src/glsl/nir/glsl_types.h b/src/glsl/nir/glsl_types.h
> index 52ca826..54e5c47 100644
> --- a/src/glsl/nir/glsl_types.h
> +++ b/src/glsl/nir/glsl_types.h
> @@ -102,6 +102,13 @@ enum glsl_matrix_layout {
>     GLSL_MATRIX_LAYOUT_ROW_MAJOR
>  };
>  
> +enum {
> +   GLSL_PRECISION_NONE = 0,
> +   GLSL_PRECISION_HIGH,
> +   GLSL_PRECISION_MEDIUM,
> +   GLSL_PRECISION_LOW
> +};
> +
>  #ifdef __cplusplus
>  #include "GL/gl.h"
>  #include "util/ralloc.h"
> @@ -834,6 +841,10 @@ struct glsl_struct_field {
>      */
>     int stream;
>  
> +   /**
> +    * Precision qualifier
> +    */
> +   unsigned precision;
>  
>     /**
>      * Image qualifiers, applicable to buffer variables defined in shader




More information about the mesa-dev mailing list