[Mesa-dev] [PATCH 1/2] glsl: Generate unique names for each const array lowered to uniforms

2014-11-18 Thread Chris Forbes
Uniform names (even for hidden uniforms) are required to be unique; some
parts of the compiler assume they can be looked up by name.

Fixes the piglit test: tests/spec/glsl-1.20/linker/array-initializers-1

Signed-off-by: Chris Forbes chr...@ijw.co.nz
Cc: 10.4 mesa-sta...@lists.freedesktop.org
---
 src/glsl/lower_const_arrays_to_uniforms.cpp | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/glsl/lower_const_arrays_to_uniforms.cpp 
b/src/glsl/lower_const_arrays_to_uniforms.cpp
index b3c0ee2..700e903 100644
--- a/src/glsl/lower_const_arrays_to_uniforms.cpp
+++ b/src/glsl/lower_const_arrays_to_uniforms.cpp
@@ -49,6 +49,7 @@ public:
{
   instructions = insts;
   progress = false;
+  index = 0;
}
 
bool run()
@@ -62,6 +63,7 @@ public:
 private:
exec_list *instructions;
bool progress;
+   unsigned index;
 };
 
 void
@@ -76,8 +78,10 @@ lower_const_array_visitor::handle_rvalue(ir_rvalue **rvalue)
 
void *mem_ctx = ralloc_parent(con);
 
+   char *uniform_name = ralloc_asprintf(mem_ctx, constarray__%d, index++);
+
ir_variable *uni =
-  new(mem_ctx) ir_variable(con-type, constarray, ir_var_uniform);
+  new(mem_ctx) ir_variable(con-type, uniform_name, ir_var_uniform);
uni-constant_initializer = con;
uni-constant_value = con;
uni-data.has_initializer = true;
-- 
2.1.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/2] glsl: Generate unique names for each const array lowered to uniforms

2014-11-18 Thread Kenneth Graunke
On Tuesday, November 18, 2014 09:15:05 PM Chris Forbes wrote:
 Uniform names (even for hidden uniforms) are required to be unique; some
 parts of the compiler assume they can be looked up by name.
 
 Fixes the piglit test: tests/spec/glsl-1.20/linker/array-initializers-1
 
 Signed-off-by: Chris Forbes chr...@ijw.co.nz
 Cc: 10.4 mesa-sta...@lists.freedesktop.org
 ---
  src/glsl/lower_const_arrays_to_uniforms.cpp | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)
 
 diff --git a/src/glsl/lower_const_arrays_to_uniforms.cpp 
 b/src/glsl/lower_const_arrays_to_uniforms.cpp
 index b3c0ee2..700e903 100644
 --- a/src/glsl/lower_const_arrays_to_uniforms.cpp
 +++ b/src/glsl/lower_const_arrays_to_uniforms.cpp
 @@ -49,6 +49,7 @@ public:
 {
instructions = insts;
progress = false;
 +  index = 0;
 }
  
 bool run()
 @@ -62,6 +63,7 @@ public:
  private:
 exec_list *instructions;
 bool progress;
 +   unsigned index;
  };
  
  void
 @@ -76,8 +78,10 @@ lower_const_array_visitor::handle_rvalue(ir_rvalue 
 **rvalue)
  
 void *mem_ctx = ralloc_parent(con);
  
 +   char *uniform_name = ralloc_asprintf(mem_ctx, constarray__%d, index++);

Yeah, I think this should work...we don't need locking on the counter because
it's not global...and we don't need to worry about getting the same name
twice because we only run the pass once (and I don't see a need to ever run it
more than once).

I had thought about doing this instead:

char *uniform_name = ralloc_asprintf(mem_ctx, constarray@%p, con);

which conveniently avoids the need for a counter.  The only bad thing is it
makes shader debug output different each invocation of the program, which
makes diffing output hard.  So, what you have is probably better.

Reviewed-by: Kenneth Graunke kenn...@whitecape.org

 +
 ir_variable *uni =
 -  new(mem_ctx) ir_variable(con-type, constarray, ir_var_uniform);
 +  new(mem_ctx) ir_variable(con-type, uniform_name, ir_var_uniform);
 uni-constant_initializer = con;
 uni-constant_value = con;
 uni-data.has_initializer = true;

signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev