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

[Jason Ekstrand]

On Aug 27, 2015 4:08 AM, "Ian Romanick" <idr at freedesktop.org> wrote:
>
> On 08/26/2015 10:11 PM, Jason Ekstrand wrote:
> > On Wed, Aug 26, 2015 at 10:20 AM, Ian Romanick <idr at freedesktop.org>
wrote:
> >> From: Ian Romanick <ian.d.romanick at intel.com>
> >>
> >> Signed-off-by: Ian Romanick <ian.d.romanick at 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?

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 <jason.ekstrand at 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 at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150827/c9660fe2/attachment-0001.html>