Mesa (master): linker: Check that initializers for global variables match
Ian Romanick
idr at kemper.freedesktop.org
Thu Nov 3 20:36:31 UTC 2011
Module: Mesa
Branch: master
Commit: f37b1ad937dd2c420f4c9fd9aa5887942bd31f3f
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=f37b1ad937dd2c420f4c9fd9aa5887942bd31f3f
Author: Ian Romanick <ian.d.romanick at intel.com>
Date: Mon Oct 31 14:31:07 2011 -0700
linker: Check that initializers for global variables match
This requires tracking a couple extra fields in ir_variable:
* A flag to indicate that a variable had an initializer.
* For non-const variables, a field to track the constant value of the
variable's initializer.
For variables non-constant initalizers, ir_variable::has_initializer
will be true, but ir_variable::constant_initializer will be NULL. The
linker can use the values of these fields to check adherence to the
GLSL 4.20 rules for shared global variables:
"If a shared global has multiple initializers, the initializers
must all be constant expressions, and they must all have the same
value. Otherwise, a link error will result. (A shared global
having only one initializer does not require that initializer to
be a constant expression.)"
Previous to 4.20 the GLSL spec simply said that initializers must have
the same value. In this case of non-constant initializers, this was
impossible to determine. As a result, no vendor actually implemented
that behavior. The 4.20 behavior matches the behavior of NVIDIA's
shipping implementations.
NOTE: This is candidate for the 7.11 branch. This patch also needs
the preceding patch "glsl: Refactor generate_ARB_draw_buffers_variables
to use add_builtin_constant"
Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=34687
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
Acked-by: Paul Berry <stereotype441 at gmail.com>
---
src/glsl/ast_to_hir.cpp | 3 ++
src/glsl/ir.cpp | 5 ++++
src/glsl/ir.h | 18 +++++++++++++++++
src/glsl/ir_clone.cpp | 5 ++++
src/glsl/ir_validate.cpp | 7 ++++++
src/glsl/ir_variable.cpp | 2 +
src/glsl/linker.cpp | 48 +++++++++++++++++++++++++++++++++++++++------
7 files changed, 81 insertions(+), 7 deletions(-)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index ebdbbc1..ed6abdc 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -2367,6 +2367,9 @@ process_initializer(ir_variable *var, ast_declaration *decl,
} else
initializer_type = rhs->type;
+ var->constant_initializer = rhs->constant_expression_value();
+ var->has_initializer = true;
+
/* If the declared variable is an unsized array, it must inherrit
* its full type from the initializer. A declaration such as
*
diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
index ef7300e..a5eca5a 100644
--- a/src/glsl/ir.cpp
+++ b/src/glsl/ir.cpp
@@ -1326,9 +1326,11 @@ ir_variable::ir_variable(const struct glsl_type *type, const char *name,
this->type = type;
this->name = ralloc_strdup(this, name);
this->explicit_location = false;
+ this->has_initializer = false;
this->location = -1;
this->warn_extension = NULL;
this->constant_value = NULL;
+ this->constant_initializer = NULL;
this->origin_upper_left = false;
this->pixel_center_integer = false;
this->depth_layout = ir_depth_layout_none;
@@ -1489,6 +1491,9 @@ steal_memory(ir_instruction *ir, void *new_ctx)
if (var != NULL && var->constant_value != NULL)
steal_memory(var->constant_value, ir);
+ if (var != NULL && var->constant_initializer != NULL)
+ steal_memory(var->constant_initializer, ir);
+
/* The components of aggregate constants are not visited by the normal
* visitor, so steal their values by hand.
*/
diff --git a/src/glsl/ir.h b/src/glsl/ir.h
index abbf455..5878c05 100644
--- a/src/glsl/ir.h
+++ b/src/glsl/ir.h
@@ -365,6 +365,14 @@ public:
unsigned explicit_location:1;
/**
+ * Does this variable have an initializer?
+ *
+ * This is used by the linker to cross-validiate initializers of global
+ * variables.
+ */
+ unsigned has_initializer:1;
+
+ /**
* \brief Layout qualifier for gl_FragDepth.
*
* This is not equal to \c ir_depth_layout_none if and only if this
@@ -414,6 +422,16 @@ public:
* Value assigned in the initializer of a variable declared "const"
*/
ir_constant *constant_value;
+
+ /**
+ * Constant expression assigned in the initializer of the variable
+ *
+ * \warning
+ * This field and \c ::constant_value are distinct. Even if the two fields
+ * refer to constants with the same value, they must point to separate
+ * objects.
+ */
+ ir_constant *constant_initializer;
};
diff --git a/src/glsl/ir_clone.cpp b/src/glsl/ir_clone.cpp
index 9adf470..e8ac9fb 100644
--- a/src/glsl/ir_clone.cpp
+++ b/src/glsl/ir_clone.cpp
@@ -50,6 +50,7 @@ ir_variable::clone(void *mem_ctx, struct hash_table *ht) const
var->origin_upper_left = this->origin_upper_left;
var->pixel_center_integer = this->pixel_center_integer;
var->explicit_location = this->explicit_location;
+ var->has_initializer = this->has_initializer;
var->num_state_slots = this->num_state_slots;
if (this->state_slots) {
@@ -68,6 +69,10 @@ ir_variable::clone(void *mem_ctx, struct hash_table *ht) const
if (this->constant_value)
var->constant_value = this->constant_value->clone(mem_ctx, ht);
+ if (this->constant_initializer)
+ var->constant_initializer =
+ this->constant_initializer->clone(mem_ctx, ht);
+
if (ht) {
hash_table_insert(ht, var, (void *)const_cast<ir_variable *>(this));
}
diff --git a/src/glsl/ir_validate.cpp b/src/glsl/ir_validate.cpp
index c387ecb..a352012 100644
--- a/src/glsl/ir_validate.cpp
+++ b/src/glsl/ir_validate.cpp
@@ -496,6 +496,13 @@ ir_validate::visit(ir_variable *ir)
}
}
+ if (ir->constant_initializer != NULL && !ir->has_initializer) {
+ printf("ir_variable didn't have an initializer, but has a constant "
+ "initializer value.\n");
+ ir->print();
+ abort();
+ }
+
return visit_continue;
}
diff --git a/src/glsl/ir_variable.cpp b/src/glsl/ir_variable.cpp
index 8fbcf1d..3092507 100644
--- a/src/glsl/ir_variable.cpp
+++ b/src/glsl/ir_variable.cpp
@@ -408,6 +408,8 @@ add_builtin_constant(exec_list *instructions, glsl_symbol_table *symtab,
name, glsl_type::int_type,
ir_var_auto, -1);
var->constant_value = new(var) ir_constant(value);
+ var->constant_initializer = new(var) ir_constant(value);
+ var->has_initializer = true;
return var;
}
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index a595c9c..915d5bb 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -448,17 +448,30 @@ cross_validate_globals(struct gl_shader_program *prog,
}
}
- /* FINISHME: Handle non-constant initializers.
+ /* Page 35 (page 41 of the PDF) of the GLSL 4.20 spec says:
+ *
+ * "If a shared global has multiple initializers, the
+ * initializers must all be constant expressions, and they
+ * must all have the same value. Otherwise, a link error will
+ * result. (A shared global having only one initializer does
+ * not require that initializer to be a constant expression.)"
+ *
+ * Previous to 4.20 the GLSL spec simply said that initializers
+ * must have the same value. In this case of non-constant
+ * initializers, this was impossible to determine. As a result,
+ * no vendor actually implemented that behavior. The 4.20
+ * behavior matches the implemented behavior of at least one other
+ * vendor, so we'll implement that for all GLSL versions.
*/
- if (var->constant_value != NULL) {
- if (existing->constant_value != NULL) {
- if (!var->constant_value->has_value(existing->constant_value)) {
+ if (var->constant_initializer != NULL) {
+ if (existing->constant_initializer != NULL) {
+ if (!var->constant_initializer->has_value(existing->constant_initializer)) {
linker_error(prog, "initializers for %s "
"`%s' have differing values\n",
mode_string(var), var->name);
return false;
}
- } else
+ } else {
/* If the first-seen instance of a particular uniform did not
* have an initializer but a later instance does, copy the
* initializer to the version stored in the symbol table.
@@ -471,8 +484,29 @@ cross_validate_globals(struct gl_shader_program *prog,
* FINISHME: modify the shader, and linking with the second
* FINISHME: will fail.
*/
- existing->constant_value =
- var->constant_value->clone(ralloc_parent(existing), NULL);
+ existing->constant_initializer =
+ var->constant_initializer->clone(ralloc_parent(existing),
+ NULL);
+ }
+ }
+
+ if (var->has_initializer) {
+ if (existing->has_initializer
+ && (var->constant_initializer == NULL
+ || existing->constant_initializer == NULL)) {
+ linker_error(prog,
+ "shared global variable `%s' has multiple "
+ "non-constant initializers.\n",
+ var->name);
+ return false;
+ }
+
+ /* Some instance had an initializer, so keep track of that. In
+ * this location, all sorts of initializers (constant or
+ * otherwise) will propagate the existence to the variable
+ * stored in the symbol table.
+ */
+ existing->has_initializer = true;
}
if (existing->invariant != var->invariant) {
More information about the mesa-commit
mailing list