Re: [Mesa-dev] [PATCH 6/8] nir: Trivial clean ups in the generated nir_constant_expressions.c

2015-12-16 Thread Kenneth Graunke
On Monday, December 14, 2015 03:34:30 PM Ian Romanick wrote:
> From: Ian Romanick 
> 
> Signed-off-by: Ian Romanick 
> ---
>  src/glsl/nir/nir_constant_expressions.py | 13 +
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/src/glsl/nir/nir_constant_expressions.py 
> b/src/glsl/nir/nir_constant_expressions.py
> index a83c12e..d82b865 100644
> --- a/src/glsl/nir/nir_constant_expressions.py
> +++ b/src/glsl/nir/nir_constant_expressions.py
> @@ -219,7 +219,6 @@ static nir_const_value
>  evaluate_${name}(unsigned num_components, nir_const_value *_src)
>  {
> nir_const_value _dst_val = { { {0, 0, 0, 0} } };
> -
> ## For each non-per-component input, create a variable srcN that
> ## contains x, y, z, and w elements which are filled in with the
> ## appropriately-typed values.
> @@ -231,7 +230,7 @@ evaluate_${name}(unsigned num_components, nir_const_value 
> *_src)
>   <% continue %>
>%endif
>  
> -  struct ${op.input_types[j]}_vec src${j} = {
> +  const struct ${op.input_types[j]}_vec src${j} = {
>% for k in range(op.input_sizes[j]):
>   % if op.input_types[j] == "bool":
>  _src[${j}].u[${k}] != 0,
> @@ -273,17 +272,17 @@ evaluate_${name}(unsigned num_components, 
> nir_const_value *_src)
> ## Avoid unused variable warnings
> <% continue %>
>  % elif op.input_types[j] == "bool":
> -   bool src${j} = _src[${j}].u[_i] != 0;
> +   const bool src${j} = _src[${j}].u[_i] != 0;
>  % else:
> -   ${op.input_types[j]} src${j} = 
> _src[${j}].${op.input_types[j][:1]}[_i];
> +   const ${op.input_types[j]} src${j} = 
> _src[${j}].${op.input_types[j][:1]}[_i];
>  % endif
>   % endfor
> -
>   ## Create an appropriately-typed variable dst and assign the
>   ## result of the const_expr to it.  If const_expr already contains
>   ## writes to dst, just include const_expr directly.
>   % if "dst" in op.const_expr:
>  ${op.output_type} dst;
> +
>  ${op.const_expr}
>   % else:
>  ${op.output_type} dst = ${op.const_expr};
> @@ -337,10 +336,8 @@ nir_eval_const_opcode(nir_op op, unsigned num_components,
>  {
> switch (op) {
>  % for name in sorted(opcodes.iterkeys()):
> -   case nir_op_${name}: {
> +   case nir_op_${name}:
>return evaluate_${name}(num_components, src);
> -  break;
> -   }
>  % endfor
> default:
>unreachable("shouldn't get here");
> 

This looks reasonable to me.

- It doesn't pollute the generator script.
- return/break is a bit silly, nice to see that go.
- marking structs as const is good.

The other consts aren't that necessary, but it won't hurt - extra const
in generated code is probably not a bad thing.

Reviewed-by: Kenneth Graunke 


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


[Mesa-dev] [PATCH 6/8] nir: Trivial clean ups in the generated nir_constant_expressions.c

2015-12-14 Thread Ian Romanick
From: Ian Romanick 

Signed-off-by: Ian Romanick 
---
 src/glsl/nir/nir_constant_expressions.py | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/src/glsl/nir/nir_constant_expressions.py 
b/src/glsl/nir/nir_constant_expressions.py
index a83c12e..d82b865 100644
--- a/src/glsl/nir/nir_constant_expressions.py
+++ b/src/glsl/nir/nir_constant_expressions.py
@@ -219,7 +219,6 @@ static nir_const_value
 evaluate_${name}(unsigned num_components, nir_const_value *_src)
 {
nir_const_value _dst_val = { { {0, 0, 0, 0} } };
-
## For each non-per-component input, create a variable srcN that
## contains x, y, z, and w elements which are filled in with the
## appropriately-typed values.
@@ -231,7 +230,7 @@ evaluate_${name}(unsigned num_components, nir_const_value 
*_src)
  <% continue %>
   %endif
 
-  struct ${op.input_types[j]}_vec src${j} = {
+  const struct ${op.input_types[j]}_vec src${j} = {
   % for k in range(op.input_sizes[j]):
  % if op.input_types[j] == "bool":
 _src[${j}].u[${k}] != 0,
@@ -273,17 +272,17 @@ evaluate_${name}(unsigned num_components, nir_const_value 
*_src)
## Avoid unused variable warnings
<% continue %>
 % elif op.input_types[j] == "bool":
-   bool src${j} = _src[${j}].u[_i] != 0;
+   const bool src${j} = _src[${j}].u[_i] != 0;
 % else:
-   ${op.input_types[j]} src${j} = 
_src[${j}].${op.input_types[j][:1]}[_i];
+   const ${op.input_types[j]} src${j} = 
_src[${j}].${op.input_types[j][:1]}[_i];
 % endif
  % endfor
-
  ## Create an appropriately-typed variable dst and assign the
  ## result of the const_expr to it.  If const_expr already contains
  ## writes to dst, just include const_expr directly.
  % if "dst" in op.const_expr:
 ${op.output_type} dst;
+
 ${op.const_expr}
  % else:
 ${op.output_type} dst = ${op.const_expr};
@@ -337,10 +336,8 @@ nir_eval_const_opcode(nir_op op, unsigned num_components,
 {
switch (op) {
 % for name in sorted(opcodes.iterkeys()):
-   case nir_op_${name}: {
+   case nir_op_${name}:
   return evaluate_${name}(num_components, src);
-  break;
-   }
 % endfor
default:
   unreachable("shouldn't get here");
-- 
2.5.0

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