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

[Bryan Cain]

On 09/10/2011 12:05 PM, Brian Paul wrote:
> 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.
>

Yes, there is a UCMP instruction on nvc0 and r600.  The float arguments
is just the src and dst registers of the MOV part of the operation.  It
could be changed to uint without making a difference.  The idea behind
keeping the MOV arguments as floats was to stay consistent with CMP, but
I can change it if it's not important.

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

I'll just change the arguments to all be uint.

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