[Mesa-dev] [PATCH] glsl: fix link error with const initializers for global variable

[Samuel Iglesias Gonsalvez]

Piglit report an error in glsl-link-initializer-03 test when
a shader without an initializer is linked in two different programs with
shaders that have differing initializers.  Linking with the first will
modify the shader, and linking with the second will fail.

To make it work, the patch detects that the none-initializer shader was
modified and tells the linker that it can link it with a new shader.

Signed-off-by: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
---
 src/glsl/ir.cpp     |  1 +
 src/glsl/ir.h       |  1 +
 src/glsl/linker.cpp | 39 ++++++++++++++++++++++++++-------------
 3 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
index 10c0006..ec452c7 100644
--- a/src/glsl/ir.cpp
+++ b/src/glsl/ir.cpp
@@ -1543,6 +1543,7 @@ ir_variable::ir_variable(const struct glsl_type *type, const char *name,
    this->warn_extension = NULL;
    this->constant_value = NULL;
    this->constant_initializer = NULL;
+   this->has_overwritten_const_initializer = false;
    this->data.origin_upper_left = false;
    this->data.pixel_center_integer = false;
    this->data.depth_layout = ir_depth_layout_none;
diff --git a/src/glsl/ir.h b/src/glsl/ir.h
index b4e52d3..c13d6f4 100644
--- a/src/glsl/ir.h
+++ b/src/glsl/ir.h
@@ -783,6 +783,7 @@ public:
     * objects.
     */
    ir_constant *constant_initializer;
+   bool has_overwritten_const_initializer;
 
 private:
    /**
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index a43d230..c6d18b3 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -711,6 +711,16 @@ cross_validate_globals(struct gl_shader_program *prog,
 	     * behavior matches the implemented behavior of at least one other
 	     * vendor, so we'll implement that for all GLSL versions.
 	     */
+
+            if (var->has_overwritten_const_initializer) {
+               /* The shader was previously modified and it didn't have any
+                * const initializer. So reset the modified fields.
+                */
+               var->has_overwritten_const_initializer = false;
+               var->constant_initializer = NULL;
+               var->data.has_initializer = false;
+            }
+
 	    if (var->constant_initializer != NULL) {
 	       if (existing->constant_initializer != NULL) {
 		  if (!var->constant_initializer->has_value(existing->constant_initializer)) {
@@ -723,20 +733,23 @@ cross_validate_globals(struct gl_shader_program *prog,
 		  /* 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.
+		   *
+		   * WARNING: The constant_value field should not be modified!
+		   *
+		   * Added has_overwritten_const_initializer to avoid
+		   * the following case: Imagine a case where a shader
+		   * without an initializer is linked in two different
+		   * programs with shaders that have differing
+		   * initializers.  Linking with the first will
+		   * modify the shader, and linking with the second
+		   * will fail.
 		   */
-		  /* 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);
-	       }
-	    }
+                  existing->constant_initializer =
+                     var->constant_initializer->clone(ralloc_parent(existing),
+                                                      NULL);
+                  existing->has_overwritten_const_initializer = true;
+               }
+            }
 
 	    if (var->data.has_initializer) {
 	       if (existing->data.has_initializer
-- 
2.0.0