[Mesa-dev] [PATCH 02/11] glsl: Add link time checks for GLSL precision qualifiers
Eduardo Lima Mitev
elima at igalia.com
Mon Jan 19 03:32:08 PST 2015
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.
No piglit regressions.
Fixes the following 4 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
---
src/glsl/ast_to_hir.cpp | 12 ++++++++++++
src/glsl/glsl_types.cpp | 4 ++++
src/glsl/glsl_types.h | 13 +++++++++++++
src/glsl/ir.h | 15 +++++++++++++++
src/glsl/linker.cpp | 48 ++++++++++++++++++++++++++++++++++++++++--------
5 files changed, 84 insertions(+), 8 deletions(-)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 811a955..2b67e14 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -2460,6 +2460,10 @@ 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 && qual->precision)
+ var->data.precision = qual->precision;
+
if (state->stage == MESA_SHADER_GEOMETRY &&
qual->flags.q.out && qual->flags.q.stream) {
var->data.stream = qual->stream;
@@ -5213,6 +5217,10 @@ ast_process_structure_or_interface_block(exec_list *instructions,
fields[i].centroid = qual->flags.q.centroid ? 1 : 0;
fields[i].sample = qual->flags.q.sample ? 1 : 0;
+ /* Precision qualifiers do not hold any meaning in Desktop GLSL */
+ fields[i].precision = state->es_shader ? qual->precision :
+ GLSL_PRECISION_NONE;
+
/* Only save explicitly defined streams in block's field */
fields[i].stream = qual->flags.q.explicit_stream ? qual->stream : -1;
@@ -5688,6 +5696,10 @@ ast_interface_block::hir(exec_list *instructions,
var->data.sample = fields[i].sample;
var->init_interface_type(block_type);
+ /* Precision qualifiers do not hold any meaning in Desktop GLSL */
+ var->data.precision = state->es_shader ? fields[i].precision :
+ GLSL_PRECISION_NONE;
+
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 b4223f4..e35c2aa 100644
--- a/src/glsl/glsl_types.cpp
+++ b/src/glsl/glsl_types.cpp
@@ -122,6 +122,7 @@ glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields,
this->fields.structure[i].centroid = fields[i].centroid;
this->fields.structure[i].sample = fields[i].sample;
this->fields.structure[i].matrix_layout = fields[i].matrix_layout;
+ this->fields.structure[i].precision = fields[i].precision;
}
mtx_unlock(&glsl_type::mutex);
@@ -668,6 +669,9 @@ glsl_type::record_compare(const glsl_type *b) const
if (this->fields.structure[i].sample
!= b->fields.structure[i].sample)
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 441015c..2405006 100644
--- a/src/glsl/glsl_types.h
+++ b/src/glsl/glsl_types.h
@@ -100,6 +100,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"
@@ -117,6 +124,7 @@ struct glsl_type {
* and \c GLSL_TYPE_UINT are valid.
*/
unsigned interface_packing:2;
+ unsigned precision:2;
/* Callers of this ralloc-based new need not call delete. It's
* easier to just ralloc_free 'mem_ctx' (or any of its ancestors). */
@@ -741,6 +749,11 @@ struct glsl_struct_field {
* streams (as in ir_variable::stream). -1 otherwise.
*/
int stream;
+
+ /**
+ * Precission qualifier
+ */
+ unsigned precision;
};
static inline unsigned int
diff --git a/src/glsl/ir.h b/src/glsl/ir.h
index a0f48b2..b17c244 100644
--- a/src/glsl/ir.h
+++ b/src/glsl/ir.h
@@ -825,6 +825,21 @@ public:
unsigned max_array_access;
/**
+ * Precision qualifier.
+ *
+ * In desktop GLSL we do not care about precision qualifiers at all, in
+ * fact the spec does not care about some incoherent uses of qualifiers
+ * that do raise an error in GLSL ES (specifically, declaring the same
+ * uniform in multiple shaders with different precision qualifiers).
+ *
+ * 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;
+
+ /**
* Allow (only) ir_variable direct access private members.
*/
friend class ir_variable;
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 3f5eac1..485b7e1 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -756,14 +756,23 @@ cross_validate_globals(struct gl_shader_program *prog,
&& existing->type->is_record()
&& existing->type->record_compare(var->type)) {
existing->type = var->type;
- } else {
- 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 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;
+ }
}
if (var->data.explicit_location) {
@@ -918,6 +927,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, probably
+ * because it basically ignores precision qualifiers. 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);
}
--
2.1.3
More information about the mesa-dev
mailing list