[LLVMdev] [Mesa-dev] [PATCH 1/2 v3] configure: add visibility macro detectionto configure

Marc Dietrich marvin24 at gmx.de
Thu Feb 26 07:26:05 PST 2015


Am Donnerstag, 26. Februar 2015, 14:44:25 schrieb Emil Velikov:
> Hi Marc
> 
> On 17/02/15 09:40, Marc Dietrich wrote:
> > 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.
> I believe this should be OK to go in regardless of the status of patch
> 2. There are just a couple of trivial nitpicks.

as pointed out, not if there is a better solution. It would be nice if people 
could test the alternative first.

> > Signed-off-by: Marc Dietrich <marvin24 at 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
> 
> I'm not sure we want/need this one ?

it restores the CFLAGS from the test above. In fact I just moved it from the 
blow upwards. Maybe the diff could be shorter.

> 
> >      # Enable -fvisibility=hidden if using a gcc that supports it
> 
> As spotted by Sedat, please update the comment.
> 
> > -    save_CFLAGS="$CFLAGS"
> > -    AC_MSG_CHECKING([whether $CC supports -fvisibility=hidden])
> > -    VISIBILITY_CFLAGS="-fvisibility=hidden"
> > -    CFLAGS="$CFLAGS $VISIBILITY_CFLAGS"
> > -    AC_LINK_IFELSE([AC_LANG_PROGRAM()], AC_MSG_RESULT([yes]),
> > -		   [VISIBILITY_CFLAGS=""; AC_MSG_RESULT([no])]);
> > -
> > -    # Restore CFLAGS; VISIBILITY_CFLAGS are added to it where needed.
> > -    CFLAGS=$save_CFLAGS
> > +    if test "x${ax_cv_have_func_attribute_visibility}" = xyes; then
> > +	VISIBILITY_CFLAGS="-fvisibility=hidden"
> > +	VISIBILITY_CXXFLAGS="-fvisibility=hidden"
> > +    fi
> 
> As these two are no longer GCC "specific" we can move them out of the if
> test x$GCC = xyes, conditional.

right.

> 
> >      # Work around aliasing bugs - developers should comment this out
> >      CFLAGS="$CFLAGS -fno-strict-aliasing"
> > 
> > @@ -267,19 +264,6 @@ fi
> > 
> >  if test "x$GXX" = xyes; then
> >  
> >      CXXFLAGS="$CXXFLAGS -Wall"
> > 
> > -    # Enable -fvisibility=hidden if using a gcc that supports it
> > -    save_CXXFLAGS="$CXXFLAGS"
> > -    AC_MSG_CHECKING([whether $CXX supports -fvisibility=hidden])
> > -    VISIBILITY_CXXFLAGS="-fvisibility=hidden"
> > -    CXXFLAGS="$CXXFLAGS $VISIBILITY_CXXFLAGS"
> > -    AC_LANG_PUSH([C++])
> > -    AC_LINK_IFELSE([AC_LANG_PROGRAM()], AC_MSG_RESULT([yes]),
> > -		   [VISIBILITY_CXXFLAGS="" ; AC_MSG_RESULT([no])]);
> > -    AC_LANG_POP([C++])
> > -
> > -    # Restore CXXFLAGS; VISIBILITY_CXXFLAGS are added to it where needed.
> > -    CXXFLAGS=$save_CXXFLAGS
> > -
> > 
> >      # Work around aliasing bugs - developers should comment this out
> >      CXXFLAGS="$CXXFLAGS -fno-strict-aliasing"
> > 
> > diff --git a/src/util/macros.h b/src/util/macros.h
> > index eec8b93..7682511 100644
> > --- a/src/util/macros.h
> > +++ b/src/util/macros.h
> > @@ -117,6 +117,12 @@ do {                       \
> > 
> >  #define PRINTFLIKE(f, a)
> >  #endif
> > 
> > +#ifdef HAVE_FUNC_ATTRIBUTE_VISIBILITY
> > +#define HIDDEN __attribute__((visibility("hidden")))
> > +#else
> > +#define HIDDEN
> > +#endif
> > +
> 
> We already define HIDDEN in the asm code.
> 
> Can you check (build or otherwise) that things don't clash for
> --enable-asm and --{en,dis}able-shared-glapi. Check the configure log,
> as we conveniently toggle the latter.

I always build with --enable-asm (or better without --disable-asm) and saw no 
symbol clash. I guess because the required header is not included at the same 
time. Is this possible at all (asm vs. c files)?

Another interesting point is in which case we can build without shared-glapi. 
I failed to build ES1 or ES2 alone, because it seem to always require glapi. 
This would mean that the dispatch table always begins with the glapi. Maybe I 
confuse things, but in this case, just using 
	extern void shared_dispatch_stub_0();
would solve all problems without the HIDDEN hacks.

Marc
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150226/870b0253/attachment.sig>


More information about the llvm-dev mailing list