[llvm-commits] [PATCH] Detect GCC atomic builtins in configure

[Eric Christopher]

On May 17, 2009, at 7:47 PM, Owen Anderson wrote:

> <config.patch>

Couple of comments:

a) the idea of testing host configuration (which may not work) for  
something target specific seems a bit problematic.

b) the indenting is a bit weird too, ok, now real stuff

c)

  AC_RUN_IFELSE(AC_LANG_SOURCE([[int main() {
+                 volatile unsigned long val = 1;
+                 __sync_synchronize();
+                 if (__sync_val_compare_and_swap(&val, 1, 0) != 1 ||
+                     val != 0)
+                   return 1;
+                 return 0; }]]),

Do you care if it runs or if it links? I'd have thought the latter,  
but it's up to you.

d)

+                AC_DEFINE(ENABLE_THREADS, [0])

You can do this using something like:

AC_DEFINE(ENABLE_THREADS, 0,
         Define if gcc atomic builtins work on your system.)

e) ENABLE_THREADS seems like a really bad name :)

f) It seems like you want to do a configure time flag? I'm not  
positive though. If you did then something like this may work better:

AC_ARG_ENABLE(enable-gcc-atomics,
[  --enable-gcc-atomics   gcc atomic functions],,
enable_gcc_atomics=no)

(or yes, depending on what you want it to default to) and then  
something like:

if test x$enable_gcc_atomics = xyes ; then
   AC_DEFINE(ENABLE_THREADS, 1,
         Define if gcc atomic builtins work on your system.)
fi

g)

+                AC_SUBST(ENABLE_THREADS, [0])

You probably don't mean to SUBST and DEFINE this. Is it for the  
environment or the compiled C/C++ files?

Sorry to seemingly pick it apart so much, but you did ask :)

-eric