Hi Sergei,
Sergei Golubchik wrote:
> Hi!
>
> On Jan 08, Davi Arnaut wrote:
>> ChangeSet@stripped, 2008-01-08 12:06:54-02:00, davi@stripped +3 -0
>> Bug#33728 Atomic builtins
>
> Basically ok. A couple of comments, see below.
> Ok to push when done.
> Good idea to use gcc builtins, thanks!
>
>> diff -Nrup a/configure.in b/configure.in
>> --- a/configure.in 2007-12-14 15:03:30 -02:00
>> +++ b/configure.in 2008-01-08 12:06:52 -02:00
>> @@ -1688,6 +1688,29 @@ case "$with_atomic_ops" in
>> *) AC_MSG_ERROR(["$with_atomic_ops" is not a valid value for
> --with-atomic-ops]) ;;
>> esac
>>
>> +AC_CACHE_CHECK([whether the compiler provides atomic builtins],
>> + [mysql_cv_atomic_builtins], [AC_TRY_RUN([
>> + int main()
>> + {
>> + int foo= -10; int bar= 10;
>> + __sync_fetch_and_add(&foo, bar);
>> + if (foo)
>> + return -1;
>> + bar= __sync_lock_test_and_set(&foo, bar);
>> + if (bar || foo != 10)
>> + return -1;
>> + bar= __sync_val_compare_and_swap(&bar, foo, 15);
>> + if (bar)
>> + return -1;
>> + return 0;
>> + }
>> +], [mysql_cv_atomic_builtins=yes], [mysql_cv_atomic_builtins=no])])
>
> add the fourth argument [ACTION-IF-CROSS-COMPILING],
> and let it be, say, no.
OK.
>> +
>> +if test "x$mysql_cv_atomic_builtins" = xyes; then
>> + AC_DEFINE(HAVE_ATOMIC_BUILTINS, 1,
>
> please rename to HAVE_GCC_ATOMIC_BUILTINS
Hum, OK. But, fwiw, it's not GCC specific.. it's also available on ICC
and other compilers.
>> + [Define to 1 if compiler provides atomic builtins.])
>> +fi
>> +
>> # Force static compilation to avoid linking problems/get more speed
>> AC_ARG_WITH(mysqld-ldflags,
>> [ --with-mysqld-ldflags Extra linking arguments for mysqld],
>> diff -Nrup a/include/atomic/builtins.h b/include/atomic/builtins.h
>> --- /dev/null Wed Dec 31 16:00:00 196900
>> +++ b/include/atomic/builtins.h 2008-01-08 12:06:52 -02:00
>
> rename the file to gcc_builtins.h please.
OK.
Thanks for the review,
Regards,
--
Davi Arnaut, Software Engineer
MySQL Inc, www.mysql.com
Are you MySQL certified? www.mysql.com/certification