On Aug 27, 2015 4:08 AM, "Ian Romanick" <<a href="mailto:idr@freedesktop.org">idr@freedesktop.org</a>> wrote: > > On 08/26/2015 10:11 PM, Jason Ekstrand wrote: > > On Wed, Aug 26, 2015 at 10:20 AM, Ian Romanick <<a href="mailto:idr@freedesktop.org">idr@freedesktop.org</a>> wrote: > >> From: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a>> > >> > >> Signed-off-by: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a>> > >> --- > >> 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 e2feff3..099bb77 100644 > >> --- a/src/glsl/nir/nir_constant_expressions.py > >> +++ b/src/glsl/nir/nir_constant_expressions.py > >> @@ -226,7 +226,6 @@ static nir_const_value > >> evaluate_${name}(unsigned num_components, nir_const_value *_src) > >> { > >> nir_const_value _dst_val = { { {0, 0, 0, 0} } }; > >> - > > > > I'd keep the blank line > > > >> ## 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. > >> @@ -238,7 +237,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, > >> @@ -280,17 +279,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 > >> - > > > > Same here. Other than that, I like the cleanup. With the blank lines kept, > > You should look at the generated code. To me, the spurious blank lines > are the most objectionable part. If someone submitted C code formatted > that way, we'd tell them to go fix it. Right? The generator, not the generated C code, is what's checked into the repo and is what people will be editing. I'd far rather keep that clean. Again just run it through indent if it bothers you. > > Reviewed-by: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>> > > > >> ## 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}; > >> @@ -344,10 +343,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.1.0 > >> > >> _______________________________________________ > >> mesa-dev mailing list > >> <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > >> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> >