List:Commits« Previous MessageNext Message »
From:Davi Arnaut Date:January 10 2008 7:09pm
Subject:Re: bk commit into 5.1 tree (davi:1.2655) BUG#33728
View as plain text  
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
Thread
bk commit into 5.1 tree (davi:1.2655) BUG#33728Davi Arnaut8 Jan
  • Re: bk commit into 5.1 tree (davi:1.2655) BUG#33728Sergei Golubchik10 Jan
    • Re: bk commit into 5.1 tree (davi:1.2655) BUG#33728Davi Arnaut10 Jan
      • Re: bk commit into 5.1 tree (davi:1.2655) BUG#33728Sergei Golubchik10 Jan