Ran into a problem with ARB_shading_language_420pack on master. I've got an app that uses non-const initializers for a matrix. In debug, the compiler aborts during IR validation. In release, it gets the incorrect answer. The below patch to an existing piglit test shows the problem. It may affect other data types, I've only tested with matrices and vectors. I did enough triage on it to figure out that process_vec_mat_constructor() is trying to index into a vector, and that doesn't get poofed away by lower_vector_insert() before validation. I don't know if the right answer is to call vector constructors or try to insert with a mask right there. Anyway, if the original authors could take a look. The current piglit tests get constant folded away. Thanks, -C diff --git a/tests/spec/arb_shading_language_420pack/execution/aggregate-initializer-matrix.shader_test b/tests/spec/arb_shading_language_420pack/execution/aggregate-initializer-matrix.shader_test index f5da4c9..0af82f1 100644 --- a/tests/spec/arb_shading_language_420pack/execution/aggregate-initializer-matrix.shader_test +++ b/tests/spec/arb_shading_language_420pack/execution/aggregate-initializer-matrix.shader_test @@ -19,9 +19,9 @@ out vec4 color; void main() { - mat2x2 a = mat2( vec2( 1.0, 0.0 ), vec2( 0.0, 1.0 ) ); - mat2x2 b = { vec2( 1.0, 0.0 ), vec2( 0.0, 1.0 ) }; - mat2x2 c = { { 1.0, 0.0 }, { 0.0, 1.0 } }; + mat2x2 a = mat2( vec2( 1.0, vertex.x ), vec2( 0.0, 1.0 ) ); + mat2x2 b = { vec2( 1.0, vertex.x ), vec2( 0.0, 1.0 ) }; + mat2x2 c = { { 1.0, vertex.x }, { 0.0, 1.0 } }; color = vec4(0.0, 1.0, 0.0, 1.0);
FWIW, I'm using the following patch locally and it works for the tests I've tried it with. No real piglit regressions (just glean noise that happens on clean driver). diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp index 4b84470..4995af4 100644 --- a/src/glsl/ast_function.cpp +++ b/src/glsl/ast_function.cpp @@ -751,10 +751,20 @@ process_vec_mat_constructor(exec_list *instructions, int i = 0; foreach_list(node, &actual_parameters) { ir_rvalue *rhs = (ir_rvalue *) node; - ir_rvalue *lhs = new(ctx) ir_dereference_array(var, - new(ctx) ir_constant(i)); + ir_instruction *assignment = NULL; + + if (var->type->is_array() || var->type->is_matrix()) { + ir_rvalue *lhs = new(ctx) ir_dereference_array(var, + new(ctx) ir_constant(i)); + assignment = new(ctx) ir_assignment(lhs, rhs, NULL); + } else { + /* use writemask rather than index for vector */ + assert(var->type->is_vector()); + assert(i < 4); + ir_dereference *lhs = new(ctx) ir_dereference_variable(var); + assignment = new(ctx) ir_assignment(lhs, rhs, NULL, (unsigned)(1 << i)); + } - ir_instruction *assignment = new(ctx) ir_assignment(lhs, rhs, NULL); instructions->push_tail(assignment); i++;
I sent the proposed fix to mesa-dev: http://www.mail-archive.com/mesa-dev@lists.freedesktop.org/msg61738.html And some additional piglit tests that start passing with the patch: http://lists.freedesktop.org/archives/piglit/2014-July/011559.html
author Cody Northrop <cody@lunarg.com> 2014-07-10 15:55:31 (GMT) committer Kenneth Graunke <kenneth@whitecape.org> 2014-07-14 15:36:36 (GMT) commit 0f679f0ab5afc8c1469453b922d37ae7216136a4 (patch) (side-by-side diff) glsl: Fix aggregates with dynamic initializers. Vectors are falling in to the ir_dereference_array() path. Without this change, the following glsl aborts the debug driver, or gets the wrong answer in release: mat2x2 a = mat2( vec2( 1.0, vertex.x ), vec2( 0.0, 1.0 ) ); Also submitting piglit tests, will reference in bug. v2: Rebase on Mesa master. v3: Remove unneeded check for arrays, which are covered by process_array_constructor(), recommended by Timothy Arceri. Signed-off-by: Cody Northrop <cody@lunarg.com> Reviewed-by: Courtney Goeltzenleuchter <courtney@lunarg.com> Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.