[Mesa-dev] [PATCH] glsl: reuse main extension table to appropriate restrict extensions

[Emil Velikov]

On 13 June 2016 at 17:16, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Mon, Jun 13, 2016 at 12:11 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> Hi Ilia,
>>
>> On 13 June 2016 at 00:23, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>
>>> @@ -81,6 +81,7 @@ MAIN_FILES = \
>>>         main/execmem.c \
>>>         main/extensions.c \
>>>         main/extensions.h \
>>> +       main/extensions_table.c \
>> From a build perspective everything looks amazing. Thank you !
>
> Thanks for checking.
>
>>
>>
>>> --- a/src/mesa/main/extensions.c
>>> +++ b/src/mesa/main/extensions.c
>>> @@ -49,25 +49,15 @@ static char *extra_extensions = NULL;
>>>  #define o(x) offsetof(struct gl_extensions, x)
>>>
>>>
>>> -/**
>>> - * \brief Table of supported OpenGL extensions for all API's.
>>> - */
>>> -const struct mesa_extension _mesa_extension_table[] = {
>>> +static bool extension_table_size[] = {
>>>  #define EXT(name_str, driver_cap, gll_ver, glc_ver, gles_ver, gles2_ver, yyyy) \
>>> -        { .name = "GL_" #name_str, .offset = o(driver_cap), \
>>> -          .version = { \
>>> -            [API_OPENGL_COMPAT] = gll_ver, \
>>> -            [API_OPENGL_CORE]   = glc_ver, \
>>> -            [API_OPENGLES]      = gles_ver, \
>>> -            [API_OPENGLES2]     = gles2_ver, \
>>> -           }, \
>>> -           .year = yyyy \
>>> -        },
>>> +   0,
>>> +
>>>  #include "extensions_table.h"
>>>  #undef EXT
>>>  };
>>>
>> An alternative idea to the one proposed by Eric:
>> Explicitly set the array size (ideally as a macro in the relevant
>> header) and use that instead of having a dummy array, only to know the
>> array size.
>
> I couldn't come up with a non-horrible way of doing that. Are you
> basically suggesting I add a
>
> #define EXTENSIONS_COUNT 10000
>
> in extensions_table.h, and expect someone to increment it every time
> an extension was being added?
>
Yes, pretty much (with the correct count of course)

> I guess it could be enforced via a STATIC_ASSERT in the new
> extensions_table.c, but it feels really dirty.
Personally a big fat warning at the top of the file + a STATIC_ASSERT
feels like a more elegant solution.

Even without the STATIC_ASSERT the compiler gives us a nice warning as
we attempt to assign more data than the actual array size.

> I guess a cleverer
> thing would be to count by doing something horrible for the compiler,
> e.g.
>
> static const int foo = 0 +1+1+1+1+1+1......
>
The following does work, although it does feel a bit nasty.

static const int foo =
#define EXT(...) 1 +
#include far.h
0;

> Probably nicer than my array of 0's though. Open to other ideas.
>
At the end of the day, it's not my call so feel free to go with
whichever, as long as others are happy.

-Emil