[Mesa-dev] [PATCH 1/2] nir: add a helper function for getting the number of source components

[Jason Ekstrand]

On Mon, Jan 26, 2015 at 2:47 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:

> On Sun, Jan 25, 2015 at 7:51 PM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> >
> >
> > On Sun, Jan 25, 2015 at 8:56 AM, Connor Abbott <cwabbott0 at gmail.com>
> wrote:
> >>
> >> Unlike with non-SSA ALU instructions, where if they're per-component
> >> you have to look at the writemask to know which source channels are
> >> being used, SSA ALU instructions always have all the possible channels
> >> enabled so we can just look at the number of components in the SSA
> >> definition for per-component instructions to say how many source
> >> components are being used.
> >>
> >> Signed-off-by: Connor Abbott <cwabbott0 at gmail.com>
> >> ---
> >>  src/glsl/nir/nir.h | 15 +++++++++++++++
> >>  1 file changed, 15 insertions(+)
> >>
> >> diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
> >> index 0ef83a1..efcaa9d 100644
> >> --- a/src/glsl/nir/nir.h
> >> +++ b/src/glsl/nir/nir.h
> >> @@ -652,6 +652,21 @@ nir_alu_instr_channel_used(nir_alu_instr *instr,
> >> unsigned src, unsigned channel)
> >>     return (instr->dest.write_mask >> channel) & 1;
> >>  }
> >>
> >> +/*
> >> + * For instructions whose destinations are SSA, get the number of
> >> channels
> >> + * used for a source
> >> + */
> >> +static inline unsigned
> >> +nir_alu_instr_ssa_src_components(nir_alu_instr *instr, unsigned src)
> >
> >
> > The name instr_ssa_src_components is kind of deceiving when the source
> isn't
> > what has to be SSA.  It's the destination that's SSA.  I'm not coming up
> > with a better name off hand, but we should try and find one.
>
> I'd like to figure out a better one too. Would it be better to put
> "ssa" earlier, i.e. nir_ssa_alu_instr_src_components to imply that the
> instruction itself has to be SSA? I would say "ssa_dest," but the name
> is already awfully long.
>

Yeah, that sounds fine.


>
> >
> > Also, There are a lot of other places that need this than just constant
> > folding.  A quick grep for input_sizes would probably reveal the ones
> that
> > are open-coding it.  Other than that, I like the series.
>
> Sure, sounds like a good idea. I wanted to get this out soon since
> it's a pretty bad bug and I was a little embarrassed not catching it
> while writing it or during review, but I'll go ahead and do that.
>

That's fair.  It can be a second patch.

With the new name, both are
Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>


>
> > --Jason
> >
> >>
> >> +{
> >> +   assert(instr->dest.dest.is_ssa);
> >> +
> >> +   if (nir_op_infos[instr->op].input_sizes[src] > 0)
> >> +      return nir_op_infos[instr->op].input_sizes[src];
> >> +
> >> +   return instr->dest.dest.ssa.num_components;
> >> +}
> >> +
> >>  typedef enum {
> >>     nir_deref_type_var,
> >>     nir_deref_type_array,
> >> --
> >> 2.1.0
> >>
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150126/89e9db59/attachment.html>