On 08/26/2015 10:11 PM, Jason Ekstrand wrote:
> On Wed, Aug 26, 2015 at 10:20 AM, Ian Romanick <i...@freedesktop.org> wrote:
>> From: Ian Romanick <ian.d.roman...@intel.com>
>>
>> Signed-off-by: Ian Romanick <ian.d.roman...@intel.com>
>> ---
>> 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?
> Reviewed-by: Jason Ekstrand <jason.ekstr...@intel.com>
>
>> ## 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
>> 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
