[Mesa-dev] [PATCH 1/4] tgsi: add a TXQS opcode to retrieve the number of texture samples

[Ilia Mirkin]

On Fri, Sep 11, 2015 at 10:42 AM, Brian Paul <brianp at vmware.com> wrote:
> On 09/10/2015 09:15 PM, Ilia Mirkin wrote:
>>
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>>   src/gallium/auxiliary/tgsi/tgsi_info.c     |  3 ++-
>>   src/gallium/docs/source/tgsi.rst           | 12 +++++++++++-
>>   src/gallium/include/pipe/p_shader_tokens.h |  1 +
>>   3 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_info.c
>> b/src/gallium/auxiliary/tgsi/tgsi_info.c
>> index fb29ea0..3b40c3d 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_info.c
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_info.c
>> @@ -141,7 +141,7 @@ static const struct tgsi_opcode_info
>> opcode_info[TGSI_OPCODE_LAST] =
>>      { 0, 0, 0, 1, 1, 0, NONE, "ENDLOOP", TGSI_OPCODE_ENDLOOP },
>>      { 0, 0, 0, 0, 1, 0, NONE, "ENDSUB", TGSI_OPCODE_ENDSUB },
>>      { 1, 1, 1, 0, 0, 0, OTHR, "TXQ_LZ", TGSI_OPCODE_TXQ_LZ },
>> -   { 0, 0, 0, 0, 0, 0, NONE, "", 104 },     /* removed */
>> +   { 1, 1, 1, 0, 0, 0, OTHR, "TXQS", TGSI_OPCODE_TXQS },
>>      { 0, 0, 0, 0, 0, 0, NONE, "", 105 },     /* removed */
>>      { 0, 0, 0, 0, 0, 0, NONE, "", 106 },     /* removed */
>>      { 0, 0, 0, 0, 0, 0, NONE, "NOP", TGSI_OPCODE_NOP },
>> @@ -331,6 +331,7 @@ tgsi_opcode_infer_type( uint opcode )
>>      case TGSI_OPCODE_SAD: /* XXX some src args may be signed for SAD ? */
>>      case TGSI_OPCODE_TXQ:
>>      case TGSI_OPCODE_TXQ_LZ:
>> +   case TGSI_OPCODE_TXQS:
>>      case TGSI_OPCODE_F2U:
>>      case TGSI_OPCODE_UDIV:
>>      case TGSI_OPCODE_UMAD:
>> diff --git a/src/gallium/docs/source/tgsi.rst
>> b/src/gallium/docs/source/tgsi.rst
>> index 314c9ca..e773e9d 100644
>> --- a/src/gallium/docs/source/tgsi.rst
>> +++ b/src/gallium/docs/source/tgsi.rst
>> @@ -960,7 +960,6 @@ XXX doesn't look like most of the opcodes really
>> belong here.
>>     For components which don't return a resource dimension, their value
>>     is undefined.
>>
>> -
>>   .. math::
>>
>>     lod = src0.x
>> @@ -973,6 +972,17 @@ XXX doesn't look like most of the opcodes really
>> belong here.
>>
>>     dst.w = texture\_levels(unit)
>>
>> +
>> +.. opcode:: TXQS - Texture Samples Query
>> +
>> +  This retrieves the number of samples in the sampler, and stores it
>
>
> I'd replace 'sampler' with 'texture', as Roland suggested.

Will do. I said 'sampler' since it takes a SAMP[] argument, not a
SVIEW[] arg. But it's a minor point, and 'texture' is definitely
clearer as to the intent.

>
>
>> +  into the x component. The other components are undefined.
>> +
>> +.. math::
>> +
>> +  dst.x = texture\_samples(unit)
>> +
>> +
>>   .. opcode:: TG4 - Texture Gather
>>
>>     As per ARB_texture_gather, gathers the four texels to be used in a
>> bi-linear
>> diff --git a/src/gallium/include/pipe/p_shader_tokens.h
>> b/src/gallium/include/pipe/p_shader_tokens.h
>> index 6e07b2c..b36e0a3 100644
>> --- a/src/gallium/include/pipe/p_shader_tokens.h
>> +++ b/src/gallium/include/pipe/p_shader_tokens.h
>> @@ -402,6 +402,7 @@ struct tgsi_property_data {
>>   #define TGSI_OPCODE_ENDLOOP             101
>>   #define TGSI_OPCODE_ENDSUB              102
>>   #define TGSI_OPCODE_TXQ_LZ              103 /* TXQ for mipmap level 0 */
>> +#define TGSI_OPCODE_TXQS                104
>>                                   /* gap */
>>   #define TGSI_OPCODE_NOP                 107
>>
>>
>
> Do you also need to add TXQS in the switch in tgsi_util.c's
> tgsi_util_get_inst_usage_mask()?  Looks like several other recently added
> opcodes (like TXQ, TXQ_LZ) aren't present there either though.

tgsi_util_get_inst_usage_mask is about usage of sources. TXQ_LZ and
TXQS don't have sources. TXQ should probably be added in there... not
sure what cares about this though.

>
> And what about an addition to tgsi_opcode_tmp.h?

Meh. When someone wants to use it with ureg, they can add it. I don't
think I've been adding stuff in there for most new opcodes. None of
the ARB_gs5 stuff is there, nor is the fp64 stuff (which Dave added).

>
> Looks good otherwise.

Thanks for looking!

  -ilia