[Mesa-dev] [PATCH 4/9] prog; add weak _mesa_error_no_memory() symbol and add it to libglsl_util

[Jose Fonseca]

On 15/04/15 20:38, Brian Paul wrote:
> On 04/15/2015 11:56 AM, Emil Velikov wrote:
>> On 15 April 2015 at 18:33, Matt Turner <mattst88 at gmail.com> wrote:
>>> On Wed, Apr 15, 2015 at 7:08 AM, Emil Velikov
>>> <emil.l.velikov at gmail.com> wrote:
>>>> Rather than forcing everyone to provide their own definition of the
>>>> symbol
>>>> provide a common weak one, which anyone can override if needed.
>>>>
>>>> This resolved the build of the standalone pipe-drivers, as it
>>>> provides a
>>>> default symbol which was missing previously.
>>>>
>>>> Cc: Rob Clark <robclark at freedesktop.org>
>>>> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
>>>> ---
>>>>   src/Makefile.am                        |  3 ++-
>>>>   src/glsl/SConscript                    |  2 ++
>>>>   src/mesa/Android.libmesa_glsl_utils.mk |  6 ++++--
>>>>   src/mesa/program/weak_errors.c         | 30
>>>> ++++++++++++++++++++++++++++++
>>>
>>> Is program/ really the right place for this? (maybe main/ instead?)
>> Hmm main/ might be better indeed. If there are any recommendations
>> about the name I'll gladly take it :)
>>
>> Jose/Brian
>>
>>  From a quick look I cannot find a suitable alternative for
>> __attribute__((weak)) for MSVC. Would you know any of the top of your
>> head or should I just wrap the attribute in ifndef _MSC_VER and leave
>> the rest ? If I drop the weak_errors.c hunk from the SCons here, and
>> replace tests/common.c with it things should just work.
>
> I'd have to do some digging to answer the MSVC question.

I don't believe MSVC has the equivalent of __attribute__((weak)).

There's _declspec(selectany) , which allows to pick any of multiple 
instances of the same symbol, but it does not allow to say which of the 
multiple implementations is strong vs weak.

> I guess I'm not totally clear on the background of what's going on here,
> but let me make a suggestion.
>
> The comment in src/gallium/state_trackers/xa/xa_tracker.c says that it's
> called by NIR, but I can't find any calls to _mesa_error_no_memory() in
> src/glsl/nir/.  I guess the actual calls are in the src/glsl/ code, right?
>
> I ran into a similar thing in the glapi code where I wanted to call some
> core Mesa function, but didn't want a dependency of that nature.
>
> The solution was to use a callback function.
>
> So, could we add a function to the glsl/nir code which registers an
> out-of-memory callback function?  The glsl/nir code could have a default
> fprintf(stderr) fallback.  But Mesa would register a callback that
> actually calls  _mesa_error() to record the error.
>
> That seems cleaner than using a compiler/linker work-around.

I agree with Brian.  This sort of compiler/linker tricks are trick to 
sustain with portable code.  Callbacks seem a much more elegant solution.

Jose