[Mesa-dev] [PATCH 15/17] glsl: Add precision information to ir_variable

Ian Romanick idr at freedesktop.org
Wed Jul 29 15:16:16 PDT 2015


On 07/29/2015 07:01 AM, Samuel Iglesias Gonsalvez 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.
> ---
>  src/glsl/ast_to_hir.cpp | 316 +++++++++++++++++++++++++++++++++++++++---------
>  src/glsl/glsl_types.cpp |   4 +
>  src/glsl/glsl_types.h   |  12 ++
>  src/glsl/ir.h           |  13 ++
>  4 files changed, 288 insertions(+), 57 deletions(-)
> 
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 789b2bc..8b170c2 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -1993,6 +1993,41 @@ process_array_type(YYLTYPE *loc, const glsl_type *base,
>     return array_type;
>  }
>  
> +static bool
> +precision_qualifier_allowed(const glsl_type *type)

This function is just moved up from below?  I would have been tempted to
put that in a separate patch to make it more obvious that there no
changes. *shrug*

> +{
> +   /* Precision qualifiers apply to floating point, integer and sampler
> +    * types.
> +    *
> +    * Section 4.5.2 (Precision Qualifiers) of the GLSL 1.30 spec says:
> +    *    "Any floating point or any integer declaration can have the type
> +    *    preceded by one of these precision qualifiers [...] Literal
> +    *    constants do not have precision qualifiers. Neither do Boolean
> +    *    variables.
> +    *
> +    * Section 4.5 (Precision and Precision Qualifiers) of the GLSL 1.30
> +    * spec also says:
> +    *
> +    *     "Precision qualifiers are added for code portability with OpenGL
> +    *     ES, not for functionality. They have the same syntax as in OpenGL
> +    *     ES."
> +    *
> +    * Section 8 (Built-In Functions) of the GLSL ES 1.00 spec says:
> +    *
> +    *     "uniform lowp sampler2D sampler;
> +    *     highp vec2 coord;
> +    *     ...
> +    *     lowp vec4 col = texture2D (sampler, coord);
> +    *                                            // texture2D returns lowp"
> +    *
> +    * 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()
> +       || type->is_integer()
> +       || type->is_record()
> +       || type->is_sampler();
> +}
>  
>  const glsl_type *
>  ast_type_specifier::glsl_type(const char **name,
> @@ -2009,31 +2044,172 @@ ast_type_specifier::glsl_type(const char **name,
>     return type;
>  }
>  
> +/**
> + * 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)
> +{
> +   switch (type->base_type) {
> +   case GLSL_TYPE_FLOAT:
> +      return "float";
> +   case GLSL_TYPE_UINT:
> +   case GLSL_TYPE_INT:
> +      return "int";
> +   case GLSL_TYPE_SAMPLER: {
> +      bool array = type->sampler_array;
> +      bool shadow = type->sampler_shadow;
> +      switch (type->sampler_type) {
> +      case GLSL_TYPE_FLOAT:
> +         switch (type->sampler_dimensionality) {
> +         case GLSL_SAMPLER_DIM_1D:
> +            if (!array && !shadow)
> +               return "sampler1D";
> +            if (array && !shadow)
> +               return "sampler1DArray";
> +            if (!array && shadow)
> +               return "sampler1DShadow";
> +            return "sampler1DArrayShadow";
> +         case GLSL_SAMPLER_DIM_2D:
> +            if (!array && !shadow)
> +               return "sampler2D";
> +            if (array && !shadow)
> +               return "sampler2DArray";
> +            if (!array && shadow)
> +               return "sampler2DShadow";
> +            return "sampler2DArrayShadow";
> +         case GLSL_SAMPLER_DIM_3D:
> +            if (!array && !shadow)
> +               return "sampler3D";
> +            assert(!"Not reached");

Use unreachable() and remove the break here and in the other places below.

I also wonder if storing array and sampler as a 2-bit value and having
an array of names in each block would be better.

    const unsigned type_idx = unsigned(type->sampler) +
       2 * unsigned(type->sampler_shadow);

    ...

     case GLSL_SAMPLER_DIM_1D: {
        static const char *const names[4] = {
           "sampler1D", "sampler1DArray",
           "sampler1DShadow", "sampler1DArrayShadow"
        };
        return names[type_idx];
     }
     ...
     case GLSL_SAMPLER_DIM_3D: {
        static const char *const names[4] = {
           "sampler3D", NULL, NULL, NULL
        };
        assert(names[type_idx] != NULL);
        return names[type_idx];
     }

Perhaps wrap all that in a macro?

#define DO_NAME(esac, name0, name1, name2, name3);  \
     case esac: {                                   \
        static const char *const names[4] = {       \
           name0, name1, name2, name3               \
        };                                          \
        assert(names[type_idx] != NULL);            \
        return names[type_idx];                     \
     }

> +            break;
> +         case GLSL_SAMPLER_DIM_CUBE:
> +            if (!array && !shadow)
> +               return "samplerCube";
> +            if (array && !shadow)
> +               return "samplercubeArray";
> +            if (!array && shadow)
> +               return "samplerCubeShadow";
> +            return "samplerCubeArrayShadow";
> +         case GLSL_SAMPLER_DIM_RECT:
> +            if (!array && !shadow)
> +               return "samplerRect";
> +            if (!array && !shadow)
> +               return "samplerRectShadow";
> +            assert(!"Not reached");
> +            break;
> +         case GLSL_SAMPLER_DIM_BUF:
> +            if (!array && !shadow)
> +               return "samplerBuffer";
> +            assert(!"Not reached");
> +            break;
> +         case GLSL_SAMPLER_DIM_EXTERNAL:
> +            if (!array && !shadow)
> +               return "samplerExternalOES";
> +            assert(!"Not reached");
> +            break;
> +         default:
> +            assert(!"Not reached");
> +         } /* sampler float dimensionality */
> +         break;
> +      case GLSL_TYPE_INT:
> +         switch (type->sampler_dimensionality) {
> +         case GLSL_SAMPLER_DIM_1D:
> +            if (!array && !shadow)
> +               return "isampler1D";
> +            if (array && !shadow)
> +               return "isampler1DArray";
> +            assert(!"Not reached");
> +            break;
> +         case GLSL_SAMPLER_DIM_2D:
> +            if (!array && !shadow)
> +               return "isampler2D";
> +            if (array && !shadow)
> +               return "isampler2DArray";
> +            assert(!"Not reached");
> +            break;
> +         case GLSL_SAMPLER_DIM_3D:
> +            if (!array && !shadow)
> +               return "isampler3D";
> +            assert(!"Not reached");
> +            break;
> +         case GLSL_SAMPLER_DIM_CUBE:
> +            if (!array && !shadow)
> +               return "isamplerCube";
> +            if (array && !shadow)
> +               return "isamplerCubeArray";
> +            assert(!"Not reached");
> +            break;
> +         default:
> +            assert(!"Not reached");
> +         } /* sampler int dimensionality */
> +         break;
> +      case GLSL_TYPE_UINT:
> +         switch (type->sampler_dimensionality) {
> +         case GLSL_SAMPLER_DIM_1D:
> +            if (!array && !shadow)
> +               return "usampler1D";
> +            if (array && !shadow)
> +               return "usampler1DArray";
> +            assert(!"Not reached");
> +            break;
> +         case GLSL_SAMPLER_DIM_2D:
> +            if (!array && !shadow)
> +               return "usampler2D";
> +            if (array && !shadow)
> +               return "usampler2DArray";
> +            assert(!"Not reached");
> +            break;
> +         case GLSL_SAMPLER_DIM_3D:
> +            if (!array && !shadow)
> +               return "usampler3D";
> +            assert(!"Not reached");
> +            break;
> +         case GLSL_SAMPLER_DIM_CUBE:
> +            if (!array && !shadow)
> +               return "usamplerCube";
> +            if (array && !shadow)
> +               return "usamplerCubeArray";
> +            assert(!"Not reached");
> +            break;
> +         default:
> +            assert(!"Not reached");
> +         } /* sampler uint dimensionality */
> +         break;
> +      default:
> +         assert(!"Not reached");
> +      } /* sampler type */
> +      break;
> +   } /* GLSL_TYPE_SAMPLER */
> +   break;
> +   default:
> +      assert(!"Not reached");
> +      break;
> +   } /* base type */
> +}
> +
>  const glsl_type *
>  ast_fully_specified_type::glsl_type(const char **name,
>                                      struct _mesa_glsl_parse_state *state) const
>  {
> -   const struct glsl_type *type = this->specifier->glsl_type(name, state);
> -
> -   if (type == NULL)
> -      return NULL;
> -
> -   /* 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.
> -    */
> -   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);
> -   }
> -
> -   return type;
> +   return this->specifier->glsl_type(name, state);
>  }
>  
>  /**
> @@ -2544,6 +2720,30 @@ apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual,
>     if (qual->flags.q.sample)
>        var->data.sample = 1;
>  
> +   /* 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 (state->es_shader) {
> +      if (qual->precision) {
> +         var->data.precision = qual->precision;
> +      } else if (precision_qualifier_allowed(var->type)) {
> +         const char *type_name =
> +            get_type_name_for_precision_qualifier(var->type);
> +         int default_precision =
> +            state->symbols->get_default_precision_qualifier(type_name);
> +         if (default_precision == ast_precision_none) {
> +            _mesa_glsl_error(loc, state,
> +                             "No precision specified in this scope for type `%s'",
> +                             var->type->name);
> +         } else {
> +            var->data.precision = default_precision;
> +         }
> +      }

This same block of code appears two more times below.  It seems like
this should be pulled out into a utility function.

> +   }
> +
>     if (state->stage == MESA_SHADER_GEOMETRY &&
>         qual->flags.q.out && qual->flags.q.stream) {
>        var->data.stream = qual->stream;
> @@ -3381,42 +3581,6 @@ validate_identifier(const char *identifier, YYLTYPE loc,
>     }
>  }
>  
> -static bool
> -precision_qualifier_allowed(const glsl_type *type)
> -{
> -   /* Precision qualifiers apply to floating point, integer and sampler
> -    * types.
> -    *
> -    * Section 4.5.2 (Precision Qualifiers) of the GLSL 1.30 spec says:
> -    *    "Any floating point or any integer declaration can have the type
> -    *    preceded by one of these precision qualifiers [...] Literal
> -    *    constants do not have precision qualifiers. Neither do Boolean
> -    *    variables.
> -    *
> -    * Section 4.5 (Precision and Precision Qualifiers) of the GLSL 1.30
> -    * spec also says:
> -    *
> -    *     "Precision qualifiers are added for code portability with OpenGL
> -    *     ES, not for functionality. They have the same syntax as in OpenGL
> -    *     ES."
> -    *
> -    * Section 8 (Built-In Functions) of the GLSL ES 1.00 spec says:
> -    *
> -    *     "uniform lowp sampler2D sampler;
> -    *     highp vec2 coord;
> -    *     ...
> -    *     lowp vec4 col = texture2D (sampler, coord);
> -    *                                            // texture2D returns lowp"
> -    *
> -    * 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()
> -       || type->is_integer()
> -       || type->is_record()
> -       || type->is_sampler();
> -}
> -
>  ir_rvalue *
>  ast_declarator_list::hir(exec_list *instructions,
>                           struct _mesa_glsl_parse_state *state)
> @@ -5672,6 +5836,25 @@ ast_process_structure_or_interface_block(exec_list *instructions,
>           fields[i].sample = qual->flags.q.sample ? 1 : 0;
>           fields[i].patch = qual->flags.q.patch ? 1 : 0;
>  
> +         /* Precision qualifiers do not hold any meaning in Desktop GLSL */
> +         if (state->es_shader) {
> +            if (qual->precision) {
> +               fields[i].precision = qual->precision;
> +            } else if (precision_qualifier_allowed(field_type)) {
> +               const char *type_name =
> +                  get_type_name_for_precision_qualifier(field_type);
> +               int default_precision =
> +                  state->symbols->get_default_precision_qualifier(type_name);
> +               if (default_precision == ast_precision_none) {
> +                  _mesa_glsl_error(&loc, state,
> +                                   "No precision specified in this scope for type `%s'",
> +                                   field_type->name);
> +               } else {
> +                  fields[i].precision = default_precision;
> +               }
> +            }
> +         }
> +
>           /* Only save explicitly defined streams in block's field */
>           fields[i].stream = qual->flags.q.explicit_stream ? qual->stream : -1;
>  
> @@ -6242,6 +6425,25 @@ 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) {
> +            if (fields[i].precision) {
> +               var->data.precision = fields[i].precision;
> +            } else if (precision_qualifier_allowed(fields[i].type)) {
> +               const char *type_name =
> +                  get_type_name_for_precision_qualifier(var->type);
> +               int default_precision =
> +                  state->symbols->get_default_precision_qualifier(type_name);
> +               if (default_precision == ast_precision_none) {
> +                  _mesa_glsl_error(&loc, state,
> +                                   "No precision specified in this scope for type `%s'",
> +                                   var->type->name);
> +               } else {
> +                  var->data.precision = default_precision;
> +               }
> +            }
> +         }
> +
>           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/glsl_types.cpp b/src/glsl/glsl_types.cpp
> index 755618a..654f22a 100644
> --- a/src/glsl/glsl_types.cpp
> +++ b/src/glsl/glsl_types.cpp
> @@ -124,6 +124,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);
> @@ -760,6 +761,9 @@ glsl_type::record_compare(const glsl_type *b) const
>        if (this->fields.structure[i].patch
>            != b->fields.structure[i].patch)
>           return false;
> +      if (this->fields.structure[i].precision
> +          != b->fields.structure[i].precision)
> +         return false;
>     }
>  
>     return true;
> diff --git a/src/glsl/glsl_types.h b/src/glsl/glsl_types.h
> index 52672b3..a4e903a 100644
> --- a/src/glsl/glsl_types.h
> +++ b/src/glsl/glsl_types.h
> @@ -101,6 +101,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"
> @@ -781,6 +788,11 @@ struct glsl_struct_field {
>      * streams (as in ir_variable::stream). -1 otherwise.
>      */
>     int stream;
> +
> +   /**
> +    * Precission qualifier
              ^ Precision
> +    */
> +   unsigned precision;
>  };
>  
>  static inline unsigned int
> diff --git a/src/glsl/ir.h b/src/glsl/ir.h
> index ede8caa..acfdfe5 100644
> --- a/src/glsl/ir.h
> +++ b/src/glsl/ir.h
> @@ -761,6 +761,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
> 



More information about the mesa-dev mailing list