On Thu, Oct 8, 2015 at 3:57 PM, Emil Velikov <emil.l.veli...@gmail.com> wrote:
> On 8 October 2015 at 19:38, Ilia Mirkin <imir...@alum.mit.edu> wrote:
>> On Thu, Oct 8, 2015 at 2:34 PM, Emil Velikov <emil.l.veli...@gmail.com>
>> wrote:
>>> On 8 October 2015 at 19:32, Ilia Mirkin <imir...@alum.mit.edu> wrote:
>>>> On Thu, Oct 8, 2015 at 2:23 PM, Emil Velikov <emil.l.veli...@gmail.com>
>>>> wrote:
>>>>> On 8 October 2015 at 18:57, Ilia Mirkin <imir...@alum.mit.edu> wrote:
>>>>>> On Thu, Oct 8, 2015 at 1:52 PM, Emil Velikov <emil.l.veli...@gmail.com>
>>>>>> wrote:
>>>>>>> On 8 October 2015 at 18:08, Ilia Mirkin <imir...@alum.mit.edu> wrote:
>>>>>>>> On Thu, Oct 8, 2015 at 1:09 PM, Emil Velikov
>>>>>>>> <emil.l.veli...@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.veli...@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 ?
>>>>>>
>>>>>> In exchange for what? From what I can tell... nothing. The array
>>>>>> definition needs to live in a c file. You can move the function to
>>>>>> look up from it into a header, that's fine.
>>>>>
>>>>> Nasty bug in(?) autohell [1]. If you're happy to dive in the auto*
>>>>> boat and fix it, I would gladly purge this series :-)
>>>>
>>>> Well that's just asking for trouble:
>>>>
>>>> src/Makefile.am: glsl/shader_enums.c \
>>>> src/mesa/Makefile.sources: ../glsl/shader_enums.c \
>>>>
>>> Yes it is nasty.
>>>
>>>> It should just be removed from shader_enums.c and make sure that
>>>> libglsl_util is a dep of libmesa_la and libmesagallium_la. Am I
>>>> missing something?
>>> Give it a go and admire the duplicated symbols :)
>>
>> Errr, that was a typo. I meant it should be removed from
>> PROGRAM_FILES. Any additional duplicated items should be removed as
>> well.
> Patches welcome :-) And please give the scons a test as well, just in case.
Looks like Rob just sent one. Good thing I was lazy about looking into it.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
