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

[Connor Abbott]

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.

>
> 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.

> --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
>>
>