[Mesa-dev] [PATCH v5 5/7] glsl: Add precision information to ir_variable
Ilia Mirkin
imirkin at alum.mit.edu
Fri Nov 13 11:37:42 PST 2015
Looks like valgrind hates this for some reason. I'm seeing lots of
==16821== Conditional jump or move depends on uninitialised value(s)
==16821== at 0xA074D09: glsl_type::record_compare(glsl_type const*)
const (glsl_types.cpp:783)
Where line 783 is:
if (this->fields.structure[i].precision
!= b->fields.structure[i].precision)
This happens with the trace from
https://bugs.freedesktop.org/show_bug.cgi?id=92229 but I suspect it
happens with just about anything with structs.
-ilia
On Wed, Nov 11, 2015 at 7:45 AM, Samuel Iglesias Gonsálvez
<siglesias at igalia.com> wrote:
> Reviewed-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
>
> On 06/11/15 13:03, 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 */
>> + 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
>>
> _______________________________________________
> 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