[Mesa-dev] [PATCH] mapi: Add better visibility checks

[Emil Velikov]

On 19/11/13 21:27, Alexander von Gluck IV wrote:
> On Tue, Nov 19, 2013 at 2:55 PM, Emil Velikov <emil.l.velikov at gmail.com>
> wrote:
>> On 17/11/13 18:11, Alexander von Gluck IV wrote:
>>>  * gl.h ensures gcc is 4.x or later before using
>>>    hidden visibility. This change matches that behaviour
>>>    and ensures better compatibility for older gcc versions.
>>>
>> Hi Alexander,
>>
>> AFAICS there are, currently, 11 cases of such ifdef spaghetti in mesa.
>>
>> IMHO we can easily compact all _but_ the following two into into a
>> single header, in a similar fashion to the inline macro (c99_compat.h).
>>
>> include/GL/gl.h
>> include/KHR/khrplatform.h -> bump the gcc version to 4?
>>
>> The gcc manual[1] indicates that the "new C++ visibility support" is
>> available as of gcc4[2]. Although for the sake of me I cannot find the
>> source of the original C visibility support. Hmm the original
>> khrplatform.h header does not have any of the GCC checks.
>>
>> Chia-I Wu would you have any idea when C visibility support was added to
>> gcc? Would you have any comment on bumping the version to 4.0 ?
>>
>> Would you mind ripping all this stuff out to a header rather than
>> carrying it around any more ?
>>
>> How do other devs feel on the subject ?
>>
> 
> Sounds good to me.  I understand Mesa defines a minimum gcc version, but
> this would increase portability of Mesa while reducing cruft.
> 
IMHO bumping the mesa gcc requirement is a bit of an overkill atm, as
most of the gcc version checks are isolated within a few header files.

include/c99/stdbool.h
include/c99_compat.h
src/mesa/main/compiler.h
src/gallium/auxiliary/util/u_atomic.h
src/gallium/auxiliary/util/u_math.h
src/gallium/include/pipe/p_compiler.h
src/gallium/include/pipe/p_config.h
src/gtest/include/gtest/internal/gtest-port.h
src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
src/mesa/main/imports.c
src/mesa/main/imports.h

Quite a few of the above are dupes due to the mesa/gallium split.

Cheers,
Emil
> I went with the 4.0 version as gcc's own webpage shows a 4.0 check. [1]
> 
>>
>>
>> [1] http://gcc.gnu.org/wiki/Visibility
>> [2] http://gcc.gnu.org/gcc-4.0/changes.html
>>
>>
>>>  ---
>>>   src/mapi/glapi/glapi.h |    2 +-
>>>   src/mapi/u_compiler.h  |    2 +-
>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>  
>>>  diff --git a/src/mapi/glapi/glapi.h b/src/mapi/glapi/glapi.h
>>>  index c764271..8e17dc7 100644
>>>  --- a/src/mapi/glapi/glapi.h
>>>  +++ b/src/mapi/glapi/glapi.h
>>>  @@ -61,7 +61,7 @@ extern "C" {
>>>   #    else
>>>   #      define _GLAPI_EXPORT __declspec(dllimport)
>>>   #    endif
>>>  -#  elif defined(__GNUC__) || (defined(__SUNPRO_C) && (__SUNPRO_C >=
>>> 0x590))
>>>  +#  elif (defined(__GNUC__) && __GNUC__ >= 4) ||
>>> (defined(__SUNPRO_C) && (__SUNPRO_C >= 0x590))
>>>   #    define _GLAPI_EXPORT __attribute__((visibility("default")))
>>>   #  else
>>>   #    define _GLAPI_EXPORT
>>>  diff --git a/src/mapi/u_compiler.h b/src/mapi/u_compiler.h
>>>  index f376e97..66c961e 100644
>>>  --- a/src/mapi/u_compiler.h
>>>  +++ b/src/mapi/u_compiler.h
>>>  @@ -11,7 +11,7 @@
>>>     /* Function visibility */
>>>   #ifndef PUBLIC
>>>  -#  if defined(__GNUC__) || (defined(__SUNPRO_C) && (__SUNPRO_C >=
>>> 0x590))
>>>  +#  if (defined(__GNUC__) && __GNUC__ >= 4) || (defined(__SUNPRO_C)
>>> && (__SUNPRO_C >= 0x590))
>>>   #    define PUBLIC __attribute__((visibility("default")))
>>>   #  elif defined(_MSC_VER)
>>>   #    define PUBLIC __declspec(dllexport)
>>>  
>>>
>>
>