Re: [PATCH] Use int64 and int32 types with GCC atomics

2016-09-16 Thread Maxim Dounin
Hello!

On Fri, Sep 16, 2016 at 02:44:38PM +0100, Alessandro Ghedini wrote:

> # HG changeset patch
> # User Alessandro Ghedini 
> # Date 1474033303 -3600
> #  Fri Sep 16 14:41:43 2016 +0100
> # Node ID 64afed0e819b9af65e7afa89baad3313f1db18d4
> # Parent  e83540f825cd8c936f4f7f1e0336279d66446606
> Use int64 and int32 types with GCC atomics
> 
> It's not quite clear if this was done on purpose, but it seems a good
> idea to use appropriately sized types when GCC atomics are used.

The "long" is expected to be one of the most native types 
available, and I see no problem with using it as long as it is 
understood by builtins used (it is).  It is also in line with the 
configure test we use to find it out if GCC atomic ops are 
present:

ngx_feature_test="long  n = 0;
  if (!__sync_bool_compare_and_swap(&n, 0, 1))
  return 1;
  if (__sync_fetch_and_add(&n, 1) != 1)
  return 1;
  if (n != 2)
  return 1;
  __sync_synchronize();"

Changing the code to diverge from the test as your patch doesn't 
looks like a good idea, and I would rather preserve this as 
is unless there are better reasons than "seems a good idea".

Note well that we anyway use the long for ngx_atomic*_t when using 
libatomic where this is the only option available.  That is, 
atomic types may not match pointer sizes, and nginx is expected to 
deal with this anyway.

-- 
Maxim Dounin
http://nginx.org/

___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel


[PATCH] Use int64 and int32 types with GCC atomics

2016-09-16 Thread Alessandro Ghedini
# HG changeset patch
# User Alessandro Ghedini 
# Date 1474033303 -3600
#  Fri Sep 16 14:41:43 2016 +0100
# Node ID 64afed0e819b9af65e7afa89baad3313f1db18d4
# Parent  e83540f825cd8c936f4f7f1e0336279d66446606
Use int64 and int32 types with GCC atomics

It's not quite clear if this was done on purpose, but it seems a good
idea to use appropriately sized types when GCC atomics are used.

diff -r e83540f825cd -r 64afed0e819b src/os/unix/ngx_atomic.h
--- a/src/os/unix/ngx_atomic.h  Thu Sep 15 15:36:02 2016 +0300
+++ b/src/os/unix/ngx_atomic.h  Fri Sep 16 14:41:43 2016 +0100
@@ -94,13 +94,18 @@
 
 #define NGX_HAVE_ATOMIC_OPS  1
 
-typedef longngx_atomic_int_t;
-typedef unsigned long   ngx_atomic_uint_t;
+#if (NGX_PTR_SIZE == 8)
 
-#if (NGX_PTR_SIZE == 8)
+typedef int64_t ngx_atomic_int_t;
+typedef uint64_tngx_atomic_uint_t;
 #define NGX_ATOMIC_T_LEN(sizeof("-9223372036854775808") - 1)
+
 #else
+
+typedef int32_t ngx_atomic_int_t;
+typedef uint32_tngx_atomic_uint_t;
 #define NGX_ATOMIC_T_LEN(sizeof("-2147483648") - 1)
+
 #endif
 
 typedef volatile ngx_atomic_uint_t  ngx_atomic_t;

___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel