[Mesa-dev] [PATCH] r600g: fix range handling for tgsi input/output declarations

[Christoph Bumiller]

On 29.03.2013 10:56, Christian König wrote:
> Am 28.03.2013 20:34, schrieb Vadim Girlin:
>> On 03/28/2013 01:01 PM, � wrote:
>>> Am 27.03.2013 20:37, schrieb Vadim Girlin:
>>>> Signed-off-by: Vadim Girlin <vadimgirlin at gmail.com>
>>>> ---
>>>>   src/gallium/drivers/r600/r600_shader.c | 19 +++++++++++++++----
>>>>   1 file changed, 15 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/src/gallium/drivers/r600/r600_shader.c
>>>> b/src/gallium/drivers/r600/r600_shader.c
>>>> index 29facf7..d4c9c03 100644
>>>> --- a/src/gallium/drivers/r600/r600_shader.c
>>>> +++ b/src/gallium/drivers/r600/r600_shader.c
>>>> @@ -874,12 +874,12 @@ static int select_twoside_color(struct
>>>> r600_shader_ctx *ctx, int front, int back
>>>>   static int tgsi_declaration(struct r600_shader_ctx *ctx)
>>>>   {
>>>>       struct tgsi_full_declaration *d =
>>>> &ctx->parse.FullToken.FullDeclaration;
>>>> -    unsigned i;
>>>> -    int r;
>>>> +    int r, i, j, count = d->Range.Last - d->Range.First + 1;
>>>>       switch (d->Declaration.File) {
>>>>       case TGSI_FILE_INPUT:
>>>> -        i = ctx->shader->ninput++;
>>>> +        i = ctx->shader->ninput;
>>>> +        ctx->shader->ninput += count;
>>>>           ctx->shader->input[i].name = d->Semantic.Name;
>>>>           ctx->shader->input[i].sid = d->Semantic.Index;
>>>>           ctx->shader->input[i].interpolate = d->Interp.Interpolate;
>>>> @@ -903,9 +903,15 @@ static int tgsi_declaration(struct
>>>> r600_shader_ctx *ctx)
>>>>                       return r;
>>>>               }
>>>>           }
>>>> +        for (j = 1; j < count; ++j) {
>>>> +            memcpy(&ctx->shader->input[i + j],
>>>> &ctx->shader->input[i],
>>>> +                   sizeof(struct r600_shader_io));
>>>
>>> Instead of memcpy, shouldn't an assignment do the trick here as well?
>>
>> Yes, assignment should work fine, I just used to use memcpy in such
>> cases for some reason. I'll replace memcpy with assignment.
>>
>> Also I think second part (outputs handling) can be dropped for now -
>> currently we only need to handle the inputs (for HUD shaders), and
>> later when array declarations for inputs/outputs will be implemented
>> in TGSI probably we'll need to update the parser in r600g anyway -
>> I'm just not sure yet how the semantic indices should be handled for
>> input/output arrays.
>>
The semantic indices are sequential, obviously. It gets more complex
with scalar arrays, but you don't have to worry about that in r600
because I'd probably add a cap for those.

Example: If you declare an "out float a[8] layout(location = k)" in GLSL
(as per ARB_separate_shader_objects), the 8 values are counted as
consuming 8 consecutive vec4 slots (here the user is responsible for
packing, nice !).
The location will be communicated via the semantic index. You'd get DCL
OUT[n..n+7] GENERIC[k] (or k+some_constant_offset because of st/mesa's
allocation policy).
If the consuming shader declares "in float b[4] layout(location = k+4)",
you'd get DCL IN[m..m+3] GENERIC[k+4], and this has to link with the
upper 4 components out the a[8] output.


>
> Yeah, the uncertainly about semantic IDs was one of the reasons I
> didn't wanted to do Input/Output arrays in the initial array
> implementation.
>
> When those changes are the only one then v2 of the patch is:
>
> Reviewed-by: Christian König <christian.koenig at amd.com>
>
> Christian.
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>