[virglrenderer-devel] [PATCH 3/4] shader: add basic shader_storage_buffer_object parsing. (v2)

[Dave Airlie]

On 18 July 2018 at 13:33, Gurchetan Singh <gurchetansingh at chromium.org> wrote:
> On Tue, Jul 17, 2018 at 7:26 PM Dave Airlie <airlied at gmail.com> wrote:
>>
>> From: Dave Airlie <airlied at redhat.com>
>>
>> This adds the basic shader parsing for the SSBO extension,
>>
>> v2: drop qualifier, cleanup unused var, add indirect support,
>> use a bitmask to track declared ssbos.
>> ---
>>  src/vrend_shader.c | 331 ++++++++++++++++++++++++++++++++++++++++++++++++-----
>>  src/vrend_shader.h |   1 +
>>  2 files changed, 305 insertions(+), 27 deletions(-)
>>
>> diff --git a/src/vrend_shader.c b/src/vrend_shader.c
>> index cbbfbbc..b20d681 100644
>> --- a/src/vrend_shader.c
>> +++ b/src/vrend_shader.c
>> @@ -108,6 +108,28 @@ struct vrend_io_range {
>>     bool used;
>>  };
>>
>> +enum vrend_type_qualifier {
>> +   TYPE_CONVERSION_NONE = 0,
>> +   FLOAT = 1,
>> +   VEC2 = 2,
>> +   VEC3 = 3,
>> +   VEC4 = 4,
>> +   INT = 5,
>> +   IVEC2 = 6,
>> +   IVEC3 = 7,
>> +   IVEC4 = 8,
>> +   UINT = 9,
>> +   UVEC2 = 10,
>> +   UVEC3 = 11,
>> +   UVEC4 = 12,
>> +   FLOAT_BITS_TO_UINT = 13,
>> +   UINT_BITS_TO_FLOAT = 14,
>> +   FLOAT_BITS_TO_INT = 15,
>> +   INT_BITS_TO_FLOAT = 16,
>> +   DOUBLE = 17,
>> +   DVEC2 = 18,
>> +};
>> +
>>  struct dump_ctx {
>>     struct tgsi_iterate_context iter;
>>     struct vrend_shader_cfg *cfg;
>> @@ -136,7 +158,12 @@ struct dump_ctx {
>>
>>     struct vrend_shader_sampler samplers[32];
>>     uint32_t samplers_used;
>> -   bool sviews_used;
>> +
>> +   uint32_t ssbo_used_mask;
>> +   uint32_t ssbo_atomic_mask;
>> +   uint32_t ssbo_array_base;
>> +   uint32_t ssbo_atomic_array_base;
>
> The v1 representation:
>
> struct vrend_ssbo {
>    uint32_t id;
>    unsigned atomic;
>    enum vrend_type_qualifier type;
> };
>
> would also allow us to fix:
>
> https://gitlab.freedesktop.org/airlied/virglrenderer/issues/3
>
> We'll have to loop over active SSBOs and change the type depending on
> opcodes (and throw an error if the typing isn't consistent).  We can
> add in the qualifier in follow-up patches, but we'll need an array
> representation ...

I think fixing that will be a thing all on it's own, I think we'd have
to have another
mask for ones that are int based instead of uint, and that would
likely cover it.

I'd rather not worry about that until after we land the initial work.

>> +         if (decl->Range.First < ctx->ssbo_atomic_array_base)
>> +            ctx->ssbo_atomic_array_base = decl->Range.First;
>> +         ctx->ssbo_atomic_mask |= (1 << decl->Range.First);
>> +      } else {
>> +         if (decl->Range.First < ctx->ssbo_array_base)
>> +            ctx->ssbo_array_base = decl->Range.First;
>
> Both ctx->ssbo_atomic_array_base and ctx->ssbo_array_base are
> decl->Range.First.  What's the difference?

We have two potential arrays, one of atomic buffers and one of ssbos,
usually we'd end up with something like
buffer[0], ATOMIC
buffer[6]
buffer[7]
in the decls, so we need to track the base for both atomic and non-atomic cases
so we get the indirect calculations correct.

Dave.