No, that's the same experiment I would have run. I was hoping the original authors could chime in, but it sounds like you have more insight via AoA work.
I'll resubmit with your change, thanks! -C On Wed, Jul 9, 2014 at 5:36 PM, Timothy Arceri <t_arc...@yahoo.com.au> wrote: > On Wed, 2014-07-02 at 22:16 +1000, Timothy Arceri wrote: > > On Tue, 2014-07-01 at 14:45 -0700, Kenneth Graunke wrote: > > > From: Cody Northrop <c...@lunarg.com> > > > > > > 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. > > > > > > Signed-off-by: Cody Northrop <c...@lunarg.com> > > > Reviewed-by: Courtney Goeltzenleuchter <court...@lunarg.com> > > > Reviewed-by: Kenneth Graunke <kenn...@whitecape.org> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79373 > > > --- > > > src/glsl/ast_function.cpp | 16 +++++++++++++--- > > > 1 file changed, 13 insertions(+), 3 deletions(-) > > > > > > Hi Cody, > > > > > > Your patch didn't apply to master due to Matt's foreach_list changes; > > > I did the obvious rebase. Otherwise, it looks great. Thanks for > fixing > > > this and improving our tests! > > > > > > I'll plan to commit this tomorrow unless anyone else has objections. > > > > > > --Ken > > > > > > diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp > > > index cdb34cc..98288d2 100644 > > > --- a/src/glsl/ast_function.cpp > > > +++ b/src/glsl/ast_function.cpp > > > @@ -743,10 +743,20 @@ process_vec_mat_constructor(exec_list > *instructions, > > > > > > int i = 0; > > > foreach_in_list(ir_rvalue, rhs, &actual_parameters) { > > > - 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()) { > > > > Is this ever actually an array??? > > I didn't mean for my question to hold this up for so long. But I cant > see where this would ever be an array, if my understanding is correct > process_array_constructor() should process any arrays. When removing > var->type->is_array() all arb_shading_language_420pack piglit tests > (including the new ones) continue to pass. So I don't think the > is_array() condition is needed. > > If I'm wrong could you please give an example of when this would be an > array as it might affect my arrays of arrays work. > > > > > > > + 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++; > > > > > > _______________________________________________ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > > > -- Cody Northrop Graphics Software Engineer LunarG, Inc.- 3D Driver Innovations Email: c...@lunarg.com Website: http://www.lunarg.com
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
