[Mesa-dev] [PATCH 1/7] glsl: move gl_shader_stage_name() definition to shader_enums.h

[Rob Clark]

On Thu, Oct 8, 2015 at 1:52 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> On 8 October 2015 at 18:08, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>> On Thu, Oct 8, 2015 at 1:09 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>> This is a trivial enough function that can live in the header. While
>>> we're here, add a STATIC_ASSERT for good measure.
>>>
>>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>>> ---
>>>  src/glsl/shader_enums.c | 13 -------------
>>>  src/glsl/shader_enums.h | 23 +++++++++++++++++++++--
>>>  2 files changed, 21 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/src/glsl/shader_enums.c b/src/glsl/shader_enums.c
>>> index c196b79..b3da3e9 100644
>>> --- a/src/glsl/shader_enums.c
>>> +++ b/src/glsl/shader_enums.c
>>> @@ -32,19 +32,6 @@
>>>  #define ENUM(x) [x] = #x
>>>  #define NAME(val) ((((val) < ARRAY_SIZE(names)) && names[(val)]) ? names[(val)] : "UNKNOWN")
>>>
>>> -const char * gl_shader_stage_name(gl_shader_stage stage)
>>> -{
>>> -   static const char *names[] = {
>>> -      ENUM(MESA_SHADER_VERTEX),
>>> -      ENUM(MESA_SHADER_TESS_CTRL),
>>> -      ENUM(MESA_SHADER_TESS_EVAL),
>>> -      ENUM(MESA_SHADER_GEOMETRY),
>>> -      ENUM(MESA_SHADER_FRAGMENT),
>>> -      ENUM(MESA_SHADER_COMPUTE),
>>> -   };
>>> -   return NAME(stage);
>>> -}
>>> -
>>>  const char * gl_vert_attrib_name(gl_vert_attrib attrib)
>>>  {
>>>     static const char *names[] = {
>>> diff --git a/src/glsl/shader_enums.h b/src/glsl/shader_enums.h
>>> index 2a5d2c5..fbd2744 100644
>>> --- a/src/glsl/shader_enums.h
>>> +++ b/src/glsl/shader_enums.h
>>> @@ -43,10 +43,27 @@ typedef enum
>>>     MESA_SHADER_COMPUTE = 5,
>>>  } gl_shader_stage;
>>>
>>> -const char * gl_shader_stage_name(gl_shader_stage stage);
>>> -
>>>  #define MESA_SHADER_STAGES (MESA_SHADER_COMPUTE + 1)
>>>
>>> +#define ENUM(x) [x] = #x
>>> +
>>> +static const char *gl_shader_stage_names[] = {
>>> +   ENUM(MESA_SHADER_VERTEX),
>>> +   ENUM(MESA_SHADER_TESS_CTRL),
>>> +   ENUM(MESA_SHADER_TESS_EVAL),
>>> +   ENUM(MESA_SHADER_GEOMETRY),
>>> +   ENUM(MESA_SHADER_FRAGMENT),
>>> +   ENUM(MESA_SHADER_COMPUTE),
>>> +};
>>
>> Won't this bloat the binary by including this in every object file? I
>> don't know that the array will get merged. (The strings will though.)
>> Normally these things live in c files so that they're stored in only
>> one object.
>>
> The linker (with its garbage collector) will sort it out for us. Seems
> like we do get a small increase from the series
>
>    text    data     bss     dec     hex filename
> 5103374  204048   27648 5335070  51681e i965_dri.so.after
> 5104590  204208   27648 5336446  516d7e i965_dri.so.before
>
> Namely - we gain ~1.2k of text. Which imho doesn't sound that bad ?

fwiw, I wanted to mention that it going to be worse than that if you
build freedreno, since many of the shader_enum to string fxns will be
called in both nir_print and in freedreno.. and probably at some point
Eric might start wanting to use them for better error msgs in vc4, so
an arm megadriver build would end up w/ 3 copies of most of the fxns
and the string tables they reference.  Likewise, I guess i965 would
get worse if it started using some of these fxns too.  That is why I
mentioned earlier that this should be at most a temporary solution.

(But moving shader_enums into NIR turned out to be relatively easy so
I guess we can avoid the static-inline temporary soln now..)

BR,
-R

> Emil
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev