[Mesa-dev] [PATCH 15/17] glsl: Add precision information to ir_variable
Iago Toral
itoral at igalia.com
Thu Jul 30 01:37:48 PDT 2015
On Wed, 2015-07-29 at 15:16 -0700, Ian Romanick wrote:
> 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];
> }
That will make the code more compact and a bit easier to read, sounds
like a good idea.
> Perhaps wrap all that in a macro?
I think this might not be a good idea. The size of the code will be more
or less the same since we will have to split the call to DO_NAME into
multiple lines anyway due to it having many parameters (the 5 you
include below plus type_idx), so there is no clear gain in that regard,
then using the macro will make things a bit more obscure I think.
> #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
> >
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list