[Mesa-dev] [PATCH 1/2] gallium: add TGSI opcodes UARL and UCMP

[Brian Paul]

On 09/10/2011 10:47 AM, Bryan Cain wrote:
> Can one of the Gallium interface maintainers please review this patch so
> I can push it?


We need documentation for these new instructions in 
src/gallium/docs/source/tgsi.rst

More comments below...

> Bryan
>
> On 09/02/2011 11:09 AM, Bryan Cain wrote:
>> They are needed by glsl_to_tgsi for an efficient implementation using native
>> integers.
>> ---
>>   src/gallium/auxiliary/tgsi/tgsi_exec.c     |   30 ++++++++++++++++++++++++++++
>>   src/gallium/auxiliary/tgsi/tgsi_info.c     |    3 ++
>>   src/gallium/include/pipe/p_shader_tokens.h |    5 +++-
>>   3 files changed, 37 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_exec.c b/src/gallium/auxiliary/tgsi/tgsi_exec.c
>> index 38dc1ef..896d871 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_exec.c
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_exec.c
>> @@ -3312,6 +3312,28 @@ micro_usne(union tgsi_exec_channel *dst,
>>   }
>>
>>   static void
>> +micro_uarl(union tgsi_exec_channel *dst,
>> +           const union tgsi_exec_channel *src)
>> +{
>> +   dst->i[0] = src->u[0];
>> +   dst->i[1] = src->u[1];
>> +   dst->i[2] = src->u[2];
>> +   dst->i[3] = src->u[3];
>> +}
>> +
>> +static void
>> +micro_ucmp(union tgsi_exec_channel *dst,
>> +           const union tgsi_exec_channel *src0,
>> +           const union tgsi_exec_channel *src1,
>> +           const union tgsi_exec_channel *src2)
>> +{
>> +   dst->f[0] = src0->u[0] ? src1->f[0] : src2->f[0];
>> +   dst->f[1] = src0->u[1] ? src1->f[1] : src2->f[1];
>> +   dst->f[2] = src0->u[2] ? src1->f[2] : src2->f[2];
>> +   dst->f[3] = src0->u[3] ? src1->f[3] : src2->f[3];
>> +}

Just curious: does UCMP directly correspond to an existing HW GPU 
instruction?  It seems a little unusual to have an instruction that 
takes a mix of float and uint arguments like that.


>> +
>> +static void
>>   exec_instruction(
>>      struct tgsi_exec_machine *mach,
>>      const struct tgsi_full_instruction *inst,
>> @@ -4071,6 +4093,14 @@ exec_instruction(
>>         assert(0);
>>         break;
>>
>> +   case TGSI_OPCODE_UARL:
>> +      exec_vector_unary(mach, inst, micro_uarl, TGSI_EXEC_DATA_INT, TGSI_EXEC_DATA_UINT);
>> +      break;
>> +
>> +   case TGSI_OPCODE_UCMP:
>> +      exec_vector_trinary(mach, inst, micro_ucmp, TGSI_EXEC_DATA_FLOAT, TGSI_EXEC_DATA_UINT);

The parameter src_datatype=TGSI_EXEC_DATA_UINT indicates that all the 
source regs are uint, but that's not what micro_ucmp() takes. 
Granted, since the float/uint values are a union and we do no 
arithmetic, UCMP would seem to work with any combination of float/uint 
arguments.  If that's the intention, please document that.


>> +      break;
>> +
>>      default:
>>         assert( 0 );
>>      }
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_info.c b/src/gallium/auxiliary/tgsi/tgsi_info.c
>> index 14ed56a..6cd580a 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_info.c
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_info.c
>> @@ -189,6 +189,9 @@ static const struct tgsi_opcode_info opcode_info[TGSI_OPCODE_LAST] =
>>      { 1, 2, 0, 0, 0, 0, "RESINFO",     TGSI_OPCODE_RESINFO },
>>      { 1, 2, 0, 0, 0, 0, "SAMPLE_POS",  TGSI_OPCODE_SAMPLE_POS },
>>      { 1, 2, 0, 0, 0, 0, "SAMPLE_INFO", TGSI_OPCODE_SAMPLE_INFO },
>> +
>> +   { 1, 1, 0, 0, 0, 0, "UARL", TGSI_OPCODE_UARL },
>> +   { 1, 3, 0, 0, 0, 0, "UCMP", TGSI_OPCODE_UCMP },
>>   };
>>
>>   const struct tgsi_opcode_info *
>> diff --git a/src/gallium/include/pipe/p_shader_tokens.h b/src/gallium/include/pipe/p_shader_tokens.h
>> index d3a3632..0a26e39 100644
>> --- a/src/gallium/include/pipe/p_shader_tokens.h
>> +++ b/src/gallium/include/pipe/p_shader_tokens.h
>> @@ -363,7 +363,10 @@ struct tgsi_property_data {
>>   #define TGSI_OPCODE_SAMPLE_POS          155
>>   #define TGSI_OPCODE_SAMPLE_INFO         156
>>
>> -#define TGSI_OPCODE_LAST                157
>> +#define TGSI_OPCODE_UARL                157
>> +#define TGSI_OPCODE_UCMP                158
>> +
>> +#define TGSI_OPCODE_LAST                159
>>
>>   #define TGSI_SAT_NONE            0  /* do not saturate */
>>   #define TGSI_SAT_ZERO_ONE        1  /* clamp to [0,1] */
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


-Brian