[Mesa-dev] [PATCH 1/2] tgsi: collect texture sampler target info in tgsi_scan_shader()

[Roland Scheidegger]

Am 29.03.2016 um 22:39 schrieb Jose Fonseca:
> On 29/03/16 21:22, Roland Scheidegger wrote:
>> I actually think "is_tex" is a bit confusing (because there's lots of
>> other parts in the code where we don't really distinguish between
>> "texture" and "sample" class of instructions and refer to them by the
>> same name), but it certainly should work here (I think originally it
>> actually meant there'd be a texture token but we have that with sample
>> instructions as well...)
>>
>> Roland
>>
>> For the series:
>> Reviewed-by: Roland Scheidegger <sroland at vmware.com>
>>
>> Am 29.03.2016 um 21:41 schrieb Brian Paul:
>>> Texture sample instructions specify a sampler unit and texture target
>>> such as "1D", "2D", "CUBE", etc.  Sampler view declarations also specify
>>> the sampler unit and texture target.
>>>
>>> This patch checks that the texture instructions agree with the
>>> declarations
>>> and collects the texture target type for each sampler unit.
>>>
>>> v2: only compare instruction's texture target to the sampler view
>>> declaration
>>> target if the instruction is a TEX instruction, not a SAMPLE
>>> instruction.
>>> ---
>>>   src/gallium/auxiliary/tgsi/tgsi_scan.c | 38
>>> ++++++++++++++++++++++++++++++++--
>>>   src/gallium/auxiliary/tgsi/tgsi_scan.h |  1 +
>>>   2 files changed, 37 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_scan.c
>>> b/src/gallium/auxiliary/tgsi/tgsi_scan.c
>>> index d32c3a1..6d4b00d 100644
>>> --- a/src/gallium/auxiliary/tgsi/tgsi_scan.c
>>> +++ b/src/gallium/auxiliary/tgsi/tgsi_scan.c
>>> @@ -181,11 +181,33 @@ scan_instruction(struct tgsi_shader_info *info,
>>>            info->indirect_files_read |= (1 << src->Register.File);
>>>         }
>>>
>>> -      /* MSAA samplers */
>>> +      /* Texture samplers */
>>>         if (src->Register.File == TGSI_FILE_SAMPLER) {
>>> +         const unsigned index = src->Register.Index;
>>> +         const unsigned target = fullinst->Texture.Texture;
>>> +
>>>            assert(fullinst->Instruction.Texture);
>>> -         assert(src->Register.Index < Elements(info->is_msaa_sampler));
>>> +         assert(index < Elements(info->is_msaa_sampler));
>>> +         assert(index < PIPE_MAX_SAMPLERS);
>>> +         assert(target < TGSI_TEXTURE_UNKNOWN);
>>> +
>>> +         if
>>> (tgsi_get_opcode_info(fullinst->Instruction.Opcode)->is_tex) {
>>> +            /* for texture instructions, check that the texture
>>> instruction
>>> +             * target matches the previous sampler view declaration
>>> (if there
>>> +             * was one.)
>>> +             */
>>> +            if (info->sampler_targets[index] == TGSI_TEXTURE_UNKNOWN) {
>>> +               /* probably no sampler view declaration */
>>> +               info->sampler_targets[index] = target;
>>> +            } else {
>>> +               /* Make sure the texture instruction's sampler/target
>>> info
>>> +                * agrees with the sampler view declaration.
>>> +                */
>>> +               assert(info->sampler_targets[index] == target);
>>> +            }
>>> +         }
>>>
>>> +         /* MSAA samplers */
>>>            if (fullinst->Instruction.Texture &&
>>>                (fullinst->Texture.Texture == TGSI_TEXTURE_2D_MSAA ||
>>>                 fullinst->Texture.Texture ==
>>> TGSI_TEXTURE_2D_ARRAY_MSAA)) {
>>> @@ -431,6 +453,16 @@ scan_declaration(struct tgsi_shader_info *info,
>>>            }
>>>         } else if (file == TGSI_FILE_SAMPLER) {
>>>            info->samplers_declared |= 1 << reg;
>>> +      } else if (file == TGSI_FILE_SAMPLER_VIEW) {
>>> +         unsigned target = fulldecl->SamplerView.Resource;
>>> +         assert(target < TGSI_TEXTURE_UNKNOWN);
>>> +         if (info->sampler_targets[reg] == TGSI_TEXTURE_UNKNOWN) {
>>> +            /* Save sampler target for this sampler index */
>>> +            info->sampler_targets[reg] = target;
>>> +         } else {
>>> +            /* if previously declared, make sure targets agree */
>>> +            assert(info->sampler_targets[reg] == target);
>>> +         }
>>>         } else if (file == TGSI_FILE_IMAGE) {
>>>            if (fulldecl->Image.Resource == TGSI_TEXTURE_BUFFER)
>>>               info->images_buffers |= 1 << reg;
>>> @@ -493,6 +525,8 @@ tgsi_scan_shader(const struct tgsi_token *tokens,
>>>      for (i = 0; i < Elements(info->const_file_max); i++)
>>>         info->const_file_max[i] = -1;
>>>      info->properties[TGSI_PROPERTY_GS_INVOCATIONS] = 1;
>>> +   for (i = 0; i < Elements(info->sampler_targets); i++)
>>> +      info->sampler_targets[i] = TGSI_TEXTURE_UNKNOWN;
>>>
>>>      /**
>>>       ** Setup to begin parsing input shader
>>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_scan.h
>>> b/src/gallium/auxiliary/tgsi/tgsi_scan.h
>>> index 76d8925..25855c8 100644
>>> --- a/src/gallium/auxiliary/tgsi/tgsi_scan.h
>>> +++ b/src/gallium/auxiliary/tgsi/tgsi_scan.h
>>> @@ -65,6 +65,7 @@ struct tgsi_shader_info
>>>      int file_max[TGSI_FILE_COUNT];  /**< highest index of declared
>>> registers */
>>>      int const_file_max[PIPE_MAX_CONSTANT_BUFFERS];
>>>      unsigned samplers_declared; /**< bitmask of declared samplers */
>>> +   ubyte sampler_targets[PIPE_MAX_SAMPLERS];  /**< TGSI_TEXTURE_x
>>> values */
>>>
>>>      ubyte input_array_first[PIPE_MAX_SHADER_INPUTS];
>>>      ubyte input_array_last[PIPE_MAX_SHADER_INPUTS];
>>>
>>
> 
> Should it be PIPE_MAX_SHADER_SAMPLER_VIEWS instead of PIPE_MAX_SAMPLERS?
> 
> It might not make difference for Mesa state tracker, but these are not
> generally the same thing.
> 
Yes you're right. (The original code actually had an implicit limit of
32 which was iffy since we need to increase that to more at some point,
but with the explicit array it really matters.)

Roland