Re: [Mesa-dev] [PATCH 3/7] glsl: add AoA support to subroutines

[Dave Airlie]

On 16 October 2015 at 12:51, Timothy Arceri  wrote:
> On Fri, 2015-10-16 at 11:44 +1000, Dave Airlie wrote:
>> you gotta give me something in the commit msg to have any idea what
>> I'm reading here :-)
>>
>> why does process_parameters move?
>
> Because we need actual_parameters processed earlier so we can use it
> with match_subroutine_by_name() to get the subroutine variable, we need
> to do this inside the recursive function generate_array_index() because
> we can't create the ir_dereference_array() until we have gotten to the
> outermost array.
>
> For the remainder of the array dimensions the type doesn't matter so we
> can just use the existing _mesa_ast_array_index_to_hir() function to
> process the ast.

Okay makes sense,

for what it's worth!
Reviewed-by: Dave Airlie 

Dave.
>
> Hope that makes sense.
>
>>
>> is there a piglit for subroutine/arrays?
>
> Just a simple one that extends one of yours tests for AoA
>
> spec/arb_arrays_of_arrays/execution/subroutines/fs
> -subroutine.shader_test
>
>>
>> Dave.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/7] glsl: add AoA support to subroutines

[Timothy Arceri]

On Wed, 2015-10-21 at 13:28 +1000, Dave Airlie wrote:
> On 16 October 2015 at 12:51, Timothy Arceri 
> wrote:
> > On Fri, 2015-10-16 at 11:44 +1000, Dave Airlie wrote:
> > > you gotta give me something in the commit msg to have any idea
> > > what
> > > I'm reading here :-)
> > > 
> > > why does process_parameters move?
> > 
> > Because we need actual_parameters processed earlier so we can use
> > it
> > with match_subroutine_by_name() to get the subroutine variable, we
> > need
> > to do this inside the recursive function generate_array_index()
> > because
> > we can't create the ir_dereference_array() until we have gotten to
> > the
> > outermost array.
> > 
> > For the remainder of the array dimensions the type doesn't matter
> > so we
> > can just use the existing _mesa_ast_array_index_to_hir() function
> > to
> > process the ast.
> 
> Okay makes sense,
> 
> for what it's worth!
> Reviewed-by: Dave Airlie 

Thanks Dave, I updated the commit msg with a similar explanation and
pushed.

> 
> Dave.
> > 
> > Hope that makes sense.
> > 
> > > 
> > > is there a piglit for subroutine/arrays?
> > 
> > Just a simple one that extends one of yours tests for AoA
> > 
> > spec/arb_arrays_of_arrays/execution/subroutines/fs
> > -subroutine.shader_test
> > 
> > > 
> > > Dave.
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/7] glsl: add AoA support to subroutines

[Timothy Arceri]

On Fri, 2015-10-16 at 11:44 +1000, Dave Airlie wrote:
> you gotta give me something in the commit msg to have any idea what
> I'm reading here :-)
> 
> why does process_parameters move?

Because we need actual_parameters processed earlier so we can use it
with match_subroutine_by_name() to get the subroutine variable, we need
to do this inside the recursive function generate_array_index() because
we can't create the ir_dereference_array() until we have gotten to the
outermost array.

For the remainder of the array dimensions the type doesn't matter so we
can just use the existing _mesa_ast_array_index_to_hir() function to
process the ast.

Hope that makes sense.

> 
> is there a piglit for subroutine/arrays?

Just a simple one that extends one of yours tests for AoA

spec/arb_arrays_of_arrays/execution/subroutines/fs
-subroutine.shader_test

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

[Mesa-dev] [PATCH 3/7] glsl: add AoA support to subroutines

[Timothy Arceri]

Cc: Dave Airlie 
---
 src/glsl/ast_function.cpp | 43 ++-
 src/glsl/lower_subroutine.cpp |  2 +-
 2 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
index c5c5cae..e4e4a3f 100644
--- a/src/glsl/ast_function.cpp
+++ b/src/glsl/ast_function.cpp
@@ -610,6 +610,37 @@ match_subroutine_by_name(const char *name,
return sig;
 }
 
+static ir_rvalue *
+generate_array_index(void *mem_ctx, exec_list *instructions,
+ struct _mesa_glsl_parse_state *state, YYLTYPE loc,
+ const ast_expression *array, ast_expression *idx,
+ const char **function_name, exec_list *actual_parameters)
+{
+   if (array->oper == ast_array_index) {
+  /* This handles arrays of arrays */
+  ir_rvalue *outer_array = generate_array_index(mem_ctx, instructions,
+state, loc,
+array->subexpressions[0],
+array->subexpressions[1],
+function_name, 
actual_parameters);
+  ir_rvalue *outer_array_idx = idx->hir(instructions, state);
+
+  YYLTYPE index_loc = idx->get_location();
+  return _mesa_ast_array_index_to_hir(mem_ctx, state, outer_array,
+  outer_array_idx, loc,
+  index_loc);
+   } else {
+  ir_variable *sub_var = NULL;
+  *function_name = array->primary_expression.identifier;
+
+  match_subroutine_by_name(*function_name, actual_parameters,
+   state, _var);
+
+  ir_rvalue *outer_array_idx = idx->hir(instructions, state);
+  return new(mem_ctx) ir_dereference_array(sub_var, outer_array_idx);
+   }
+}
+
 static void
 print_function_prototypes(_mesa_glsl_parse_state *state, YYLTYPE *loc,
   ir_function *f)
@@ -1989,16 +2020,18 @@ ast_function_expression::hir(exec_list *instructions,
   ir_variable *sub_var = NULL;
   ir_rvalue *array_idx = NULL;
 
+  process_parameters(instructions, _parameters, >expressions,
+state);
+
   if (id->oper == ast_array_index) {
- func_name = id->subexpressions[0]->primary_expression.identifier;
-array_idx = id->subexpressions[1]->hir(instructions, state);
+ array_idx = generate_array_index(ctx, instructions, state, loc,
+  id->subexpressions[0],
+  id->subexpressions[1], _name,
+  _parameters);
   } else {
  func_name = id->primary_expression.identifier;
   }
 
-  process_parameters(instructions, _parameters, >expressions,
-state);
-
   ir_function_signature *sig =
 match_function_by_name(func_name, _parameters, state);
 
diff --git a/src/glsl/lower_subroutine.cpp b/src/glsl/lower_subroutine.cpp
index c1aed61..a0df5e1 100644
--- a/src/glsl/lower_subroutine.cpp
+++ b/src/glsl/lower_subroutine.cpp
@@ -84,7 +84,7 @@ lower_subroutine_visitor::visit_leave(ir_call *ir)
  continue;
 
   if (ir->array_idx != NULL)
- var = new(mem_ctx) ir_dereference_array(ir->sub_var, 
ir->array_idx->clone(mem_ctx, NULL));
+ var = ir->array_idx->clone(mem_ctx, NULL);
   else
  var = new(mem_ctx) ir_dereference_variable(ir->sub_var);
 
-- 
2.4.3

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

Re: [Mesa-dev] [PATCH 3/7] glsl: add AoA support to subroutines

[Dave Airlie]

you gotta give me something in the commit msg to have any idea what
I'm reading here :-)

why does process_parameters move?

is there a piglit for subroutine/arrays?

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