[PATCH 0/2 v3] add visibility hidden to tls entry points

Patch 1 adds a check for the compilers visibility macro to configure.ac.
Patch 2 avoids redefined symbol errors in clang of the tls entry points.
Based on a suggestion from Rafael Ávila de Espíndola <rafael.espindola@gmail.com>
in http://llvm.org/bugs/show_bug.cgi?id=19778.

Tested with gcc 4.9 and clang 3.6(rc)

Marc Dietrich (2):
  configure: add visibility macro detection to configure
  add visibility hidden to tls entry points

configure.ac | 28 ++++++----------------------
src/mapi/Makefile.am | 1 +
src/mapi/entry_x86-64_tls.h | 4 ++--
src/mapi/entry_x86_tls.h | 5 +++--
src/mapi/entry_x86_tsd.h | 5 +++--
src/util/macros.h | 6 ++++++
6 files changed, 21 insertions(+), 28 deletions(-)

This adds clang/gcc visibility macro detection to configure and util/macros.h.
This is can be used to conveniently add e.g. a "HIDDEN" attribute to a function.

Signed-off-by: Marc Dietrich <marvin24@gmx.de>

Avoid redefined symbol errors in clang. Based on a suggestion from
Rafael Ávila de Espíndola <rafael.espindola@gmail.com> in
http://llvm.org/bugs/show_bug.cgi?id=19778.

Signed-off-by: Marc Dietrich <marvin24@gmx.de>

This adds clang/gcc visibility macro detection to configure and util/macros.h.
This is can be used to conveniently add e.g. a "HIDDEN" attribute to a function.

Signed-off-by: Marc Dietrich <marvin24@gmx.de>
---
v2: use VISIBILITY_*FLAGS instead of *FLAGS directly
v3: no change

configure.ac | 28 ++++++----------------------
src/util/macros.h | 6 ++++++
2 files changed, 12 insertions(+), 22 deletions(-)

diff --git a/configure.ac b/configure.ac
index 351027b..266764a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -189,6 +189,7 @@ AX_GCC_FUNC_ATTRIBUTE([flatten])
AX_GCC_FUNC_ATTRIBUTE([format])
AX_GCC_FUNC_ATTRIBUTE([malloc])
AX_GCC_FUNC_ATTRIBUTE([packed])
+AX_GCC_FUNC_ATTRIBUTE([visibility])

AM_CONDITIONAL([GEN_ASM_OFFSETS], test "x$GEN_ASM_OFFSETS" = xyes)

@@ -245,17 +246,13 @@ if test "x$GCC" = xyes; then
                   AC_MSG_RESULT([yes]),
                   [CFLAGS="$save_CFLAGS -Wmissing-prototypes";
                    AC_MSG_RESULT([no])]);
+ CFLAGS=$save_CFLAGS

     # Enable -fvisibility=hidden if using a gcc that supports it

Can you restore this comment with a general pointer to "...compiler
that support it".

BTW, what is the compiler option called for clang to suppress
visibility(-hiden) feature - ... -fvisibility=no ?
I would like to test w/o your patches.

- Sedat -

Avoid redefined symbol errors in clang. Based on a suggestion from
Rafael Ávila de Espíndola <rafael.espindola@gmail.com> in
http://llvm.org/bugs/show_bug.cgi?id=19778.

Signed-off-by: Marc Dietrich <marvin24@gmx.de>
---
v2: - no change
v3: - include util directory in Makefile.am in order to avoid relative
      include paths,
    - make tls entry points array const

I adapted this patch as v4 to fit mesa v10.4.4, but it fails (see
log-file in attached tarball).

- Sedat -

for-marvin24.tar.gz (12.1 KB)

for-marvin24.tar.gz.sha256sum (86 Bytes)

the configure output says that visibility is not supported by your clang.
Interestingly, it is supported here (also clang 3.6 from yesterday). What does
config.log say about it?

Marc

Unfortunately, I have killed the build-dir and trying the patch from FreeBSD.
I will rebuild and send you my config.log.

Attached is my build-script for my llvm-toolchain.

- Sedat -

build_llvm-toolchain.sh (1.32 KB)

mmh, no compiler-rt?

NO.
Can you explain why do I need it?

- Sedat -

No, I was just following

http://clang.llvm.org/get_started.html

where it looks like it is required. That's the only difference between your
and my setup AFAICS.

Thanks for the hint!
This means re-compiling my llvm-toolchain :-(.

- Sedat -

Here is my config.log.

$ grep -i visibility config.log
configure:18401: checking for __attribute__((visibility))
ax_cv_have_func_attribute_visibility=no
VISIBILITY_CFLAGS=''
VISIBILITY_CXXFLAGS=''

- Sedat -

config.log (318 KB)

the problem here is that you are using "-v" flag which generates some warning
output. The result is that *all* attribute detection fails. Remove it and try
again.

Marc

>> > Avoid redefined symbol errors in clang. Based on a suggestion from
>> > Rafael Ávila de Espíndola <rafael.espindola@gmail.com> in
>> > http://llvm.org/bugs/show_bug.cgi?id=19778.
>> >
>> > Signed-off-by: Marc Dietrich <marvin24@gmx.de>
>> > ---
>> > v2: - no change
>> > v3: - include util directory in Makefile.am in order to avoid relative
>> >
>> > include paths,
>> >
>> > - make tls entry points array const
>>
>> I adapted this patch as v4 to fit mesa v10.4.4, but it fails (see
>> log-file in attached tarball).
>
> the configure output says that visibility is not supported by your clang.
> Interestingly, it is supported here (also clang 3.6 from yesterday). What
> does config.log say about it?

Here is my config.log.

$ grep -i visibility config.log
configure:18401: checking for __attribute__((visibility))
ax_cv_have_func_attribute_visibility=no
VISIBILITY_CFLAGS=''
VISIBILITY_CXXFLAGS=''

the problem here is that you are using "-v" flag which generates some warning
output. The result is that *all* attribute detection fails. Remove it and try
again.

Looks good, now.

$ grep -i visibility config.log
configure:18401: checking for __attribute__((visibility))

#define HAVE_FUNC_ATTRIBUTE_VISIBILITY 1
#define HAVE_FUNC_ATTRIBUTE_VISIBILITY 1
#define HAVE_FUNC_ATTRIBUTE_VISIBILITY 1
#define HAVE_FUNC_ATTRIBUTE_VISIBILITY 1
#define HAVE_FUNC_ATTRIBUTE_VISIBILITY 1
#define HAVE_FUNC_ATTRIBUTE_VISIBILITY 1
#define HAVE_FUNC_ATTRIBUTE_VISIBILITY 1
#define HAVE_FUNC_ATTRIBUTE_VISIBILITY 1

ax_cv_have_func_attribute_visibility=yes
DEFS='-DPACKAGE_NAME=\"Mesa\" -DPACKAGE_TARNAME=\"mesa\"
-DPACKAGE_VERSION=\"10.4.4\" -DPACKAGE_STRING=\"Mesa\ 10.4.4\"
-DPACKAGE_BUGREPORT=\"https://bugs.freedesktop.org/enter_bug.cgi\?product=Mesa\"
-DPACKAGE_URL=\"\" -DPACKAGE=\"mesa\" -DVERSION=\"10.4.4\"
-DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1
-DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1
-DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1
-DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 -DLT_OBJDIR=\".libs/\"
-DYYTEXT_POINTER=1 -DHAVE___BUILTIN_BSWAP32=1
-DHAVE___BUILTIN_BSWAP64=1 -DHAVE___BUILTIN_CLZ=1
-DHAVE___BUILTIN_CLZLL=1 -DHAVE___BUILTIN_CTZ=1
-DHAVE___BUILTIN_EXPECT=1 -DHAVE___BUILTIN_FFS=1
-DHAVE___BUILTIN_FFSLL=1 -DHAVE___BUILTIN_POPCOUNT=1
-DHAVE___BUILTIN_POPCOUNTLL=1 -DHAVE___BUILTIN_UNREACHABLE=1
-DHAVE_FUNC_ATTRIBUTE_FLATTEN=1 -DHAVE_FUNC_ATTRIBUTE_FORMAT=1
-DHAVE_FUNC_ATTRIBUTE_MALLOC=1 -DHAVE_FUNC_ATTRIBUTE_PACKED=1
-DHAVE_FUNC_ATTRIBUTE_VISIBILITY=1 -DHAVE_DLADDR=1 -DHAVE_PTHREAD=1
-DHAVE_LIBEXPAT=1'
VISIBILITY_CFLAGS='-fvisibility=hidden'
VISIBILITY_CXXFLAGS='-fvisibility=hidden'
#define HAVE_FUNC_ATTRIBUTE_VISIBILITY 1

- Sedat -

Patch 1 adds a check for the compilers visibility macro to configure.ac.
Patch 2 avoids redefined symbol errors in clang of the tls entry points.
Based on a suggestion from Rafael Ávila de Espíndola <rafael.espindola@gmail.com>
in http://llvm.org/bugs/show_bug.cgi?id=19778.

Tested with gcc 4.9 and clang 3.6(rc)

Marc Dietrich (2):
  configure: add visibility macro detection to configure
  add visibility hidden to tls entry points

configure.ac | 28 ++++++----------------------
src/mapi/Makefile.am | 1 +
src/mapi/entry_x86-64_tls.h | 4 ++--
src/mapi/entry_x86_tls.h | 5 +++--
src/mapi/entry_x86_tsd.h | 5 +++--
src/util/macros.h | 6 ++++++
6 files changed, 21 insertions(+), 28 deletions(-)

Tested-by: Sedat Dilek <sedat.dilek@gmail.com> (mesa v10.4.4 with
llvm-toolchain v3.6.0rc2)

- Sedat -

v3 of my build-script reflects the prereq of compiler-rt.

- Sedat -

build_llvm-toolchain-v3.sh (1.53 KB)

Here the fix for the comment in visibility macro detection.

- Sedat -

0001-configure-Fix-comment-in-visibility-macro-detection.patch (943 Bytes)

BTW, I would change the subject to something like...

   "configure: add visibility macro detection" (or s/add/introduce)

Can you send a v4 of your patchset, please?
With appropriate credits (Reported-by/Tested-by of mine)?

Thanks!

- Sedat -

Tested-by: Sedat Dilek <sedat.dilek@gmail.com> (mesa v10.4.4 with
llvm-toolchain v3.6.0rc*3* with compiler-rt)

I re-compiled my Linux graphics driver stack with a self-compiled
llvm-toolchain v3.6.0rc3.

libdrm: v2.4.59
mesa: v10.4.4 (plus gallivm-fixes and visibility-macro-detection
support) + configure: --enable-glx-tls
intel-ddx: v2.99.917-149-g09b0ab9b4384

What about renaming your patches to...?

1/2: "configure: add visibility macro detection"
2/2: "mapi: add visibility hidden to tls entry points"

Even 1/2 should reflect it's a code-generation option for compilers
like GCC or LLVM/Clang.
It's up2u.
Please have a look at [1], too.

Thanks.

- Sedat -

[1] Code Gen Options (Using the GNU Compiler Collection (GCC)) ->
"-fvisibility=[default|internal|hidden|protected]"

build-and-install-log_mesa-10-4-4_llvm-3-6-0-rc3_gallivm-fixes_visibility-macro-detection_enable-glx-tls.txt.gz (21.3 KB)

build-and-install-log_mesa-10-4-4_llvm-3-6-0-rc3_gallivm-fixes_visibility-macro-detection_enable-glx-tls.txt.gz.sha256sum (178 Bytes)

0001-gallivm-Update-for-RTDyldMemoryManager-becoming-an-u.patch (1.12 KB)

0002-configure-add-visibility-macro-detection-to-configur.patch (3 KB)

0003-add-visibility-hidden-to-tls-entry-points.patch (2.66 KB)

0004-configure-Fix-comment-in-visibility-macro-detection.patch (947 Bytes)

Ping!?

- Sedat -