On Friday, May 6, 2016 10:55:36 AM PDT Juan A. Suarez Romero wrote:
> Currently, when cross validating global variables, all global variables
> seen in the shaders that are part of a program are saved in a table.
>
> When checking a variable this already exist in the table, we check both
> are initialized to the same value. If the already saved variable does
> not have an initializer, we copy it from the new variable.
>
> Unfortunately this is wrong, as we are modifying something it is
> constant. Also, if this modified variable is used in
> another program, it will keep the initializer, when it should have none.
>
> Instead of copying the initializer, this commit replaces the old
> variable with the new one. So if we see again the same variable with an
> initializer, we can compare if both are the same or not.
> ---
> src/compiler/glsl/glsl_symbol_table.cpp | 10 ++++++++++
> src/compiler/glsl/glsl_symbol_table.h | 5 +++++
> src/compiler/glsl/linker.cpp | 27 +++++----------------------
> 3 files changed, 20 insertions(+), 22 deletions(-)
>
> diff --git a/src/compiler/glsl/glsl_symbol_table.cpp b/src/compiler/glsl/
glsl_symbol_table.cpp
> index 6c682ac..6d7baad 100644
> --- a/src/compiler/glsl/glsl_symbol_table.cpp
> +++ b/src/compiler/glsl/glsl_symbol_table.cpp
> @@ -278,3 +278,13 @@ glsl_symbol_table::disable_variable(const char *name)
> entry->v = NULL;
> }
> }
> +
> +void
> +glsl_symbol_table::replace_variable(const char *name,
> + ir_variable *v)
> +{
> + symbol_table_entry *entry = get_entry(name);
> + if (entry != NULL) {
> + entry->v = v;
> + }
> +}
> diff --git a/src/compiler/glsl/glsl_symbol_table.h b/src/compiler/glsl/
glsl_symbol_table.h
> index 5d654e5..2f94d4c 100644
> --- a/src/compiler/glsl/glsl_symbol_table.h
> +++ b/src/compiler/glsl/glsl_symbol_table.h
> @@ -100,6 +100,11 @@ struct glsl_symbol_table {
> */
> void disable_variable(const char *name);
>
> + /**
> + * Replaces the variable in the entry by the new variable.
> + */
> + void replace_variable(const char *name, ir_variable *v);
> +
> private:
> symbol_table_entry *get_entry(const char *name);
>
> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> index 9c72478..4522734 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -1093,21 +1093,11 @@ cross_validate_globals(struct gl_shader_program
*prog,
> return;
> }
> } 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.
> - */
> - /* FINISHME: This is wrong. The constant_value field should
> - * FINISHME: not be modified! Imagine a case where a shader
> - * FINISHME: without an initializer is linked in two different
> - * FINISHME: programs with shaders that have differing
> - * FINISHME: initializers. Linking with the first will
> - * FINISHME: modify the shader, and linking with the second
> - * FINISHME: will fail.
> - */
> - existing->constant_initializer =
> - var->constant_initializer->clone(ralloc_parent(existing),
> - NULL);
> + /* If the first-seen instance of a particular uniform did
> + * not have an initializer but a later instance does,
> + * replace the former with the later.
> + */
Please convert the tabs in the above lines to spaces.
Thanks for fixing this!
Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
> + variables.replace_variable(existing->name, var);
> }
> }
>
> @@ -1121,13 +1111,6 @@ cross_validate_globals(struct gl_shader_program
*prog,
> var->name);
> return;
> }
> -
> - /* 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->data.has_initializer = true;
> }
>
> if (existing->data.invariant != var->data.invariant) {
>
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
