public inbox for libc-hacker@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] atomic.h fixes + testcase, misc fixes
@ 2003-03-21 20:39 Jakub Jelinek
  2003-03-22  0:11 ` Roland McGrath
  2003-03-22 23:52 ` Roland McGrath
  0 siblings, 2 replies; 4+ messages in thread
From: Jakub Jelinek @ 2003-03-21 20:39 UTC (permalink / raw)
  To: Ulrich Drepper, Roland McGrath; +Cc: Glibc hackers

Hi!

I found a couple of issues with various atomic.h headers and decide
to write a testcase for all the atomic.h macros (it doesn't test if the
operations are actually atomic, just whether they do what they are supposed
to do). This revealed a bunch of other bugs.

It is unclear to me what the semantics of a couple of macros should be,
as the include/atomic.h version is different from i486 and x86-64 version.
The testcase passes on IA-64 and i386 (using sysdeps/generic/bits/atomic.h),
but there are failures on i686.
atomic_increment_and_test test 1 failed
atomic_decrement_and_test test 1 failed
atomic_add_negative test 1 failed
atomic_add_zero test 2 failed

IMHO every macro in include/atomic.h should have a comment explaining
what it exactly does and then tst-atomic.c should be changed to match that.
E.g. atomic_increment_and_test seems to test the value after increment
while generic version tests the value before increment.

Another problem are the nested statement expressions. We must ensure
we never use the same names for local variables in them if they can be
nested. This patch fixes one where things would expand into
__typeof(something) __oldvalue = (__oldvalue);
and another one where similar thing happens with __memp.
I wonder whether we shouldn't use some unique prefixes for these local
variables, say __ followed by first letters of word in macro name __
and descriptive name or something.

2003-03-21  Jakub Jelinek  <jakub@redhat.com>

	* include/atomic.h (atomic_compare_and_exchange_val_acq): Add comment.
	Don't define if __arch_compare_and_exchange_val_32_acq is not defined.
	(atomic_compare_and_exchange_bool_acq): Add comment.  Don't use
	__oldval variable in the macro, since it might be macro argument.
	(atomic_decrement_if_positive): Initialize __memp, remove setting
	of non-existent variable.
	(atomic_bit_test_set): Cast 1 to __typeof (*mem) before shifting.
	* sysdeps/ia64/bits/atomic.h (atomic_exchange_and_add): Implement
	using atomic_compare_and_exchange_val_acq.
	(atomic_decrement_if_positive, atomic_bit_test_set): Define.
	* sysdeps/s390/bits/atomic.h (__arch_compare_and_exchange_val_8_acq):
	Renamed from...
	(__arch_compare_and_exchange_bool_8_acq): ... this.
	(__arch_compare_and_exchange_val_16_acq): Renamed from...
	(__arch_compare_and_exchange_bool_16_acq): ... this.
	(__arch_compare_and_exchange_val_32_acq): Return old value.  Renamed
	from...
	(__arch_compare_and_exchange_bool_32_acq): ... this.
	(__arch_compare_and_exchange_val_64_acq): Return old value.  Renamed
	from...
	(__arch_compare_and_exchange_bool_64_acq): ... this.
	* sysdeps/generic/bits/atomic.h (arch_compare_and_exchange_acq):
	Remove.
	(atomic_compare_and_exchange_val_acq,
	atomic_compare_and_exchange_bool_acq): Define.
	* sysdeps/x86_64/bits/atomic.h (atomic_bit_set): Shift 1L instead
	of 1 up in the 64-bit case.
	* csu/tst-atomic.c: New test.
	* csu/tst-atomic-long.c: New test.
	* csu/Makefile (tests): Add tst-atomic and tst-atomic-long.

	* malloc/memusagestat.c (main): Kill warning if uint64_t is ulong.

	* sysdeps/s390/Versions: Add trailing newline.

	* sysdeps/unix/sysv/linux/sysconf.c (__sysconf): Kill warning
	if INTERNAL_SYSCALL_ERROR_P doesn't use its first argument.

--- libc/csu/tst-atomic.c.jj	2003-03-21 10:09:42.000000000 -0500
+++ libc/csu/tst-atomic.c	2003-03-21 11:41:50.000000000 -0500
@@ -0,0 +1,276 @@
+/* Copyright (C) 2003 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by Jakub Jelinek <jakub@redhat.com>, 2003.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, write to the Free
+   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+   02111-1307 USA.  */
+
+#include <stdio.h>
+#include <atomic.h>
+
+#ifndef atomic_t
+# define atomic_t int
+#endif
+
+/* Test various atomic.h macros.  */
+static int
+do_test (void)
+{
+  atomic_t mem;
+  int ret = 0;
+
+#ifdef atomic_compare_and_exchange_val_acq
+  mem = 24;
+  if (atomic_compare_and_exchange_val_acq (&mem, 35, 24) != 24
+      || mem != 35)
+    {
+      puts ("atomic_compare_and_exchange_val_acq test 1 failed");
+      ret = 1;
+    }
+
+  mem = 12;
+  if (atomic_compare_and_exchange_val_acq (&mem, 10, 15) != 12
+      || mem != 12)
+    {
+      puts ("atomic_compare_and_exchange_val_acq test 2 failed");
+      ret = 1;
+    }
+#endif
+
+  mem = 24;
+  if (atomic_compare_and_exchange_bool_acq (&mem, 35, 24)
+      || mem != 35)
+    {
+      puts ("atomic_compare_and_exchange_bool_acq test 1 failed");
+      ret = 1;
+    }
+
+  mem = 12;
+  if (! atomic_compare_and_exchange_bool_acq (&mem, 10, 15)
+      || mem != 12)
+    {
+      puts ("atomic_compare_and_exchange_bool_acq test 2 failed");
+      ret = 1;
+    }
+
+  mem = 64;
+  if (atomic_exchange (&mem, 31) != 64
+      || mem != 31)
+    {
+      puts ("atomic_exchange test failed");
+      ret = 1;
+    }
+
+  mem = 2;
+  if (atomic_exchange_and_add (&mem, 11) != 2
+      || mem != 13)
+    {
+      puts ("atomic_exchange_and_add test failed");
+      ret = 1;
+    }
+
+  mem = -21;
+  atomic_add (&mem, 22);
+  if (mem != 1)
+    {
+      puts ("atomic_add test failed");
+      ret = 1;
+    }
+
+  mem = -1;
+  atomic_increment (&mem);
+  if (mem != 0)
+    {
+      puts ("atomic_increment test failed");
+      ret = 1;
+    }
+
+  mem = 0;
+  if (! atomic_increment_and_test (&mem)
+      || mem != 1)
+    {
+      puts ("atomic_increment_and_test test 1 failed");
+      ret = 1;
+    }
+
+  mem = 35;
+  if (atomic_increment_and_test (&mem)
+      || mem != 36)
+    {
+      puts ("atomic_increment_and_test test 2 failed");
+      ret = 1;
+    }
+
+  mem = 17;
+  atomic_decrement (&mem);
+  if (mem != 16)
+    {
+      puts ("atomic_decrement test failed");
+      ret = 1;
+    }
+
+  mem = 0;
+  if (! atomic_decrement_and_test (&mem)
+      || mem != -1)
+    {
+      puts ("atomic_decrement_and_test test 1 failed");
+      ret = 1;
+    }
+
+  mem = 15;
+  if (atomic_decrement_and_test (&mem)
+      || mem != 14)
+    {
+      puts ("atomic_decrement_and_test test 2 failed");
+      ret = 1;
+    }
+
+  mem = 1;
+  if (atomic_decrement_if_positive (&mem) != 1
+      || mem != 0)
+    {
+      puts ("atomic_decrement_if_positive test 1 failed");
+      ret = 1;
+    }
+
+  mem = 0;
+  if (atomic_decrement_if_positive (&mem) != 0
+      || mem != 0)
+    {
+      puts ("atomic_decrement_if_positive test 2 failed");
+      ret = 1;
+    }
+
+  mem = -1;
+  if (atomic_decrement_if_positive (&mem) != -1
+      || mem != -1)
+    {
+      puts ("atomic_decrement_if_positive test 3 failed");
+      ret = 1;
+    }
+
+  mem = -10;
+  if (! atomic_add_negative (&mem, 12)
+      || mem != 2)
+    {
+      puts ("atomic_add_negative test 1 failed");
+      ret = 1;
+    }
+
+  mem = 0;
+  if (atomic_add_negative (&mem, 100)
+      || mem != 100)
+    {
+      puts ("atomic_add_negative test 2 failed");
+      ret = 1;
+    }
+
+  mem = 15;
+  if (atomic_add_negative (&mem, -10)
+      || mem != 5)
+    {
+      puts ("atomic_add_negative test 3 failed");
+      ret = 1;
+    }
+
+  mem = -34;
+  if (atomic_add_zero (&mem, 31)
+      || mem != -3)
+    {
+      puts ("atomic_add_zero test 1 failed");
+      ret = 1;
+    }
+
+  mem = 0;
+  if (! atomic_add_zero (&mem, 36)
+      || mem != 36)
+    {
+      puts ("atomic_add_zero test 2 failed");
+      ret = 1;
+    }
+
+  mem = 113;
+  if (atomic_add_zero (&mem, -13)
+      || mem != 100)
+    {
+      puts ("atomic_add_zero test 3 failed");
+      ret = 1;
+    }
+
+  mem = 0;
+  atomic_bit_set (&mem, 1);
+  if (mem != 2)
+    {
+      puts ("atomic_bit_set test 1 failed");
+      ret = 1;
+    }
+
+  mem = 8;
+  atomic_bit_set (&mem, 3);
+  if (mem != 8)
+    {
+      puts ("atomic_bit_set test 2 failed");
+      ret = 1;
+    }
+
+#ifdef TEST_ATOMIC64
+  mem = 16;
+  atomic_bit_set (&mem, 35);
+  if (mem != 0x800000010LL)
+    {
+      puts ("atomic_bit_set test 3 failed");
+      ret = 1;
+    }
+#endif
+
+  mem = 0;
+  if (atomic_bit_test_set (&mem, 1)
+      || mem != 2)
+    {
+      puts ("atomic_bit_test_set test 1 failed");
+      ret = 1;
+    }
+
+  mem = 8;
+  if (! atomic_bit_test_set (&mem, 3)
+      || mem != 8)
+    {
+      puts ("atomic_bit_test_set test 2 failed");
+      ret = 1;
+    }
+
+#ifdef TEST_ATOMIC64
+  mem = 16;
+  if (atomic_bit_test_set (&mem, 35)
+      || mem != 0x800000010LL)
+    {
+      puts ("atomic_bit_test_set test 3 failed");
+      ret = 1;
+    }
+
+  mem = 0x100000000LL;
+  if (! atomic_bit_test_set (&mem, 32)
+      || mem != 0x100000000LL)
+    {
+      puts ("atomic_bit_test_set test 4 failed");
+      ret = 1;
+    }
+#endif
+
+  return 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
--- libc/csu/Makefile.jj	2002-12-31 21:33:13.000000000 -0500
+++ libc/csu/Makefile	2003-03-21 11:23:40.000000000 -0500
@@ -1,5 +1,5 @@
 # Makefile for csu code for GNU C library.
-# Copyright (C) 1995,96,97,98,99,2000,01,2002 Free Software Foundation, Inc.
+# Copyright (C) 1995,96,97,98,99,2000,01,02,2003 Free Software Foundation, Inc.
 # This file is part of the GNU C Library.
 
 # The GNU C Library is free software; you can redistribute it and/or
@@ -42,6 +42,8 @@ distribute = initfini.c gmon-start.c sta
 generated = version-info.h
 before-compile = $(objpfx)version-info.h
 
+tests := tst-atomic tst-atomic-long
+
 all: # Make this the default target; it will be defined in Rules.
 
 include ../Makeconfig
--- libc/csu/tst-atomic-long.c.jj	2003-03-21 11:17:56.000000000 -0500
+++ libc/csu/tst-atomic-long.c	2003-03-21 11:21:33.000000000 -0500
@@ -0,0 +1,27 @@
+/* Copyright (C) 2003 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by Jakub Jelinek <jakub@redhat.com>, 2003.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, write to the Free
+   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+   02111-1307 USA.  */
+
+#include <bits/wordsize.h>
+
+#define atomic_t long
+#if __WORDSIZE == 64
+# define TEST_ATOMIC64 1
+#endif
+
+#include "tst-atomic.c"
--- libc/include/atomic.h.jj	2003-03-21 05:17:25.000000000 -0500
+++ libc/include/atomic.h	2003-03-21 11:45:04.000000000 -0500
@@ -25,7 +25,10 @@
 #include <bits/atomic.h>
 
 
-#ifndef atomic_compare_and_exchange_val_acq
+/* Atomically store NEWVAL in *MEM if *MEM is equal to OLDVAL.
+   Return the old *MEM value.  */
+#if !defined atomic_compare_and_exchange_val_acq \
+    && defined __arch_compare_and_exchange_val_32_acq
 # define atomic_compare_and_exchange_val_acq(mem, newval, oldval) \
   ({ __typeof (*mem) __result;						      \
      if (sizeof (*mem) == 1)						      \
@@ -48,6 +51,8 @@
 #endif
 
 
+/* Atomically store NEWVAL in *MEM if *MEM is equal to OLDVAL.
+   Return zero if *MEM was changed or non-zero if no exchange happened.  */
 #ifndef atomic_compare_and_exchange_bool_acq
 # ifdef __arch_compare_and_exchange_bool_32_acq
 #  define atomic_compare_and_exchange_bool_acq(mem, newval, oldval) \
@@ -69,8 +74,10 @@
      __result; })
 # else
 #  define atomic_compare_and_exchange_bool_acq(mem, newval, oldval) \
-  ({ __typeof (oldval) __oldval = (oldval);				      \
-     atomic_compare_and_exchange_val_acq (mem, newval, __oldval) != __oldval; \
+  ({ /* Cannot use __oldval here, because macros later in this file might     \
+	call this macro with __oldval argument.	 */			      \
+     __typeof (oldval) __old = (oldval);				      \
+     atomic_compare_and_exchange_val_acq (mem, newval, __old) != __old;	      \
   })
 # endif
 #endif
@@ -150,9 +157,8 @@
 #ifndef atomic_decrement_if_positive
 # define atomic_decrement_if_positive(mem) \
   ({ __typeof (*mem) __oldval;						      \
-     __typeof (mem) __memp;						      \
+     __typeof (mem) __memp = (mem);					      \
 									      \
-     __val = *__memp;							      \
      do									      \
        {								      \
 	 __oldval = *__memp;						      \
@@ -190,7 +196,7 @@
 # define atomic_bit_test_set(mem, bit) \
   ({ __typeof (*mem) __oldval;						      \
      __typeof (mem) __memp = (mem);					      \
-     __typeof (*mem) __mask = (1 << (bit));				      \
+     __typeof (*mem) __mask = ((__typeof (*mem)) 1 << (bit));		      \
 									      \
      do									      \
        __oldval = (*__memp);						      \
--- libc/malloc/memusagestat.c.jj	2001-08-23 12:48:21.000000000 -0400
+++ libc/malloc/memusagestat.c	2003-03-21 09:29:53.000000000 -0500
@@ -405,7 +405,7 @@ main (int argc, char *argv[])
     }
 
 
-  snprintf (buf, sizeof (buf), "%llu", total);
+  snprintf (buf, sizeof (buf), "%llu", (unsigned long long) total);
   gdImageString (im_out, gdFontSmall, xsize - 50, ysize - 14, buf, blue);
 
   if (!time_based)
--- libc/sysdeps/generic/bits/atomic.h.jj	2003-03-20 01:56:50.000000000 -0500
+++ libc/sysdeps/generic/bits/atomic.h	2003-03-21 12:32:06.000000000 -0500
@@ -25,7 +25,19 @@
    up with real definitions.  */
 
 /* The only basic operation needed is compare and exchange.  */
-#define arch_compare_and_exchange_acq(mem, newval, oldval) \
-  ({ *(mem) == (oldval) ? 1 : (*(mem) = (newval), 0); })
+#define atomic_compare_and_exchange_val_acq(mem, newval, oldval) \
+  ({ __typeof (mem) __gmemp = (mem);				      \
+     __typeof (*mem) __gret = *__gmemp;				      \
+     __typeof (*mem) __gnewval = (newval);			      \
+								      \
+     if (__gret == (oldval))					      \
+       *__gmemp = __gnewval;					      \
+     __gret; })
+
+#define atomic_compare_and_exchange_bool_acq(mem, newval, oldval) \
+  ({ __typeof (mem) __gmemp = (mem);				      \
+     __typeof (*mem) __gnewval = (newval);			      \
+								      \
+     *__gmemp == (oldval) ? (*__gmemp = __gnewval, 0) : 1; })
 
 #endif	/* bits/atomic.h */
--- libc/sysdeps/ia64/bits/atomic.h.jj	2003-03-21 05:17:32.000000000 -0500
+++ libc/sysdeps/ia64/bits/atomic.h	2003-03-21 11:13:16.000000000 -0500
@@ -78,30 +78,49 @@ typedef uintmax_t uatomic_max_t;
   __sync_lock_test_and_set_si (mem, value)
 
 #define atomic_exchange_and_add(mem, value) \
-  ({									      \
-    __typeof (*mem) __oldval, __val;					      \
-    __typeof (mem) __memp = (mem);					      \
-    __typeof (*mem) __value = (value);					      \
+  ({ __typeof (*mem) __oldval, __val;					      \
+     __typeof (mem) __memp = (mem);					      \
+     __typeof (*mem) __value = (value);					      \
 									      \
-    __val = *__memp;							      \
-    if (sizeof (*mem) == 4)						      \
-      do								      \
-	{								      \
-	  __oldval = __val;						      \
-	  __val = __arch_compare_and_exchange_32_val_acq (__memp,	      \
-							  __oldval + __value, \
-							  __oldval);	      \
-	}								      \
-      while (__builtin_expect (__val != __oldval, 0));			      \
-    else if (sizeof (*mem) == 8)					      \
-      do								      \
-	{								      \
-	  __oldval = __val;						      \
-	  __val = __arch_compare_and_exchange_64_val_acq (__memp,	      \
-							  __oldval + __value, \
-							  __oldval);	      \
-	}								      \
-      while (__builtin_expect (__val != __oldval, 0));			      \
-    else								      \
-      abort ();								      \
-    __oldval + __value; })
+     __val = (*__memp);							      \
+     do									      \
+       {								      \
+	 __oldval = __val;						      \
+	 __val = atomic_compare_and_exchange_val_acq (__memp,		      \
+						      __oldval + __value,     \
+						      __oldval);	      \
+       }								      \
+     while (__builtin_expect (__val != __oldval, 0));			      \
+     __oldval; })
+
+#define atomic_decrement_if_positive(mem) \
+  ({ __typeof (*mem) __oldval, __val;					      \
+     __typeof (mem) __memp = (mem);					      \
+									      \
+     __val = (*__memp);							      \
+     do									      \
+       {								      \
+	 __oldval = __val;						      \
+	 if (__builtin_expect (__val <= 0, 0))				      \
+	   break;							      \
+	 __val = atomic_compare_and_exchange_val_acq (__memp,	__oldval - 1, \
+						      __oldval);	      \
+       }								      \
+     while (__builtin_expect (__val != __oldval, 0));			      \
+     __oldval; })
+
+#define atomic_bit_test_set(mem, bit) \
+  ({ __typeof (*mem) __oldval, __val;					      \
+     __typeof (mem) __memp = (mem);					      \
+     __typeof (*mem) __mask = ((__typeof (*mem)) 1 << (bit));		      \
+									      \
+     __val = (*__memp);							      \
+     do									      \
+       {								      \
+	 __oldval = __val;						      \
+	 __val = atomic_compare_and_exchange_val_acq (__memp,		      \
+						      __oldval | __mask,      \
+						      __oldval);	      \
+       }								      \
+     while (__builtin_expect (__val != __oldval, 0));			      \
+     __oldval & __mask; })
--- libc/sysdeps/s390/bits/atomic.h.jj	2003-03-21 05:17:34.000000000 -0500
+++ libc/sysdeps/s390/bits/atomic.h	2003-03-21 08:23:32.000000000 -0500
@@ -45,34 +45,32 @@ typedef intmax_t atomic_max_t;
 typedef uintmax_t uatomic_max_t;
 
 
-#define __arch_compare_and_exchange_bool_8_acq(mem, newval, oldval) \
+#define __arch_compare_and_exchange_val_8_acq(mem, newval, oldval) \
   (abort (), 0)
 
-#define __arch_compare_and_exchange_bool_16_acq(mem, newval, oldval) \
+#define __arch_compare_and_exchange_val_16_acq(mem, newval, oldval) \
   (abort (), 0)
 
-#define __arch_compare_and_exchange_bool_32_acq(mem, newval, oldval) \
+#define __arch_compare_and_exchange_val_32_acq(mem, newval, oldval) \
   ({ unsigned int *__mem = (unsigned int *) (mem);			      \
      unsigned int __old = (unsigned int) (oldval);			      \
-     unsigned int __cmp = __old;					      \
      __asm __volatile ("cs %0,%2,%1"					      \
 		       : "+d" (__old), "=Q" (*__mem)			      \
 		       : "d" (newval), "m" (*__mem) : "cc" );		      \
-     __cmp != __old; })
+     __old; })
 
 #ifdef __s390x__
-# define __arch_compare_and_exchange_bool_64_acq(mem, newval, oldval) \
+# define __arch_compare_and_exchange_val_64_acq(mem, newval, oldval) \
   ({ unsigned long int *__mem = (unsigned long int *) (mem);		      \
      unsigned long int __old = (unsigned long int) (oldval);		      \
-     unsigned long int __cmp = __old;					      \
      __asm __volatile ("csg %0,%2,%1"					      \
 		       : "+d" (__old), "=Q" (*__mem)			      \
 		       : "d" (newval), "m" (*__mem) : "cc" );		      \
-     __cmp != __old; })
+     __old; })
 #else
 /* For 31 bit we do not really need 64-bit compare-and-exchange. We can
    implement them by use of the csd instruction. The straightforward
    implementation causes warnings so we skip the definition for now.  */
-# define __arch_compare_and_exchange_bool_64_acq(mem, newval, oldval) \
+# define __arch_compare_and_exchange_val_64_acq(mem, newval, oldval) \
   (abort (), 0)
 #endif
--- libc/sysdeps/s390/Versions.jj	2003-01-28 05:32:49.000000000 -0500
+++ libc/sysdeps/s390/Versions	2003-03-21 09:24:20.000000000 -0500
@@ -3,4 +3,4 @@ ld {
     # runtime interface to TLS
     __tls_get_offset;
   }
-}
\ No newline at end of file
+}
--- libc/sysdeps/unix/sysv/linux/sysconf.c.jj	2003-03-21 09:33:38.000000000 -0500
+++ libc/sysdeps/unix/sysv/linux/sysconf.c	2003-03-21 09:36:34.000000000 -0500
@@ -40,7 +40,8 @@ __sysconf (int name)
       {
 	struct timespec ts;
 	INTERNAL_SYSCALL_DECL (err);
-	int r = INTERNAL_SYSCALL (clock_getres, err, 2, CLOCK_MONOTONIC, &ts);
+	int r;
+	r = INTERNAL_SYSCALL (clock_getres, err, 2, CLOCK_MONOTONIC, &ts);
 	return INTERNAL_SYSCALL_ERROR_P (r, err) ? 0 : 1;
       }
 #endif
--- libc/sysdeps/x86_64/bits/atomic.h.jj	2003-03-21 05:17:40.000000000 -0500
+++ libc/sysdeps/x86_64/bits/atomic.h	2003-03-21 10:00:50.000000000 -0500
@@ -291,7 +291,7 @@ typedef uintmax_t uatomic_max_t;
 	    else							      \
 	      __asm __volatile (LOCK "orq %q2, %0"			      \
 				: "=m" (*mem)				      \
-				: "m" (*mem), "i" (1 << (bit)));	      \
+				: "m" (*mem), "i" (1L << (bit)));	      \
 	    })
 
 

	Jakub

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] atomic.h fixes + testcase, misc fixes
  2003-03-21 20:39 [PATCH] atomic.h fixes + testcase, misc fixes Jakub Jelinek
@ 2003-03-22  0:11 ` Roland McGrath
  2003-03-22  0:56   ` Jakub Jelinek
  2003-03-22 23:52 ` Roland McGrath
  1 sibling, 1 reply; 4+ messages in thread
From: Roland McGrath @ 2003-03-22  0:11 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Ulrich Drepper, Glibc hackers

Is there any reason not to use inlines instead of macros?
That would avoid the shadowed variable issues.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] atomic.h fixes + testcase, misc fixes
  2003-03-22  0:11 ` Roland McGrath
@ 2003-03-22  0:56   ` Jakub Jelinek
  0 siblings, 0 replies; 4+ messages in thread
From: Jakub Jelinek @ 2003-03-22  0:56 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Ulrich Drepper, Glibc hackers

On Fri, Mar 21, 2003 at 04:08:45PM -0800, Roland McGrath wrote:
> Is there any reason not to use inlines instead of macros?
> That would avoid the shadowed variable issues.

Yes. The macros work with any types, while for inlines you'd
have to use special _int, _long etc. versions.

	Jakub

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] atomic.h fixes + testcase, misc fixes
  2003-03-21 20:39 [PATCH] atomic.h fixes + testcase, misc fixes Jakub Jelinek
  2003-03-22  0:11 ` Roland McGrath
@ 2003-03-22 23:52 ` Roland McGrath
  1 sibling, 0 replies; 4+ messages in thread
From: Roland McGrath @ 2003-03-22 23:52 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Ulrich Drepper, Glibc hackers

I've put in these changes and your later patch.  I made some more tweaks,
though things are still all consistent.  I am not really sure what the best
semantics for all the macros with boolean results are.  It depends on the
usage and the cheapness on processors where it makes a difference.  So far
only atomic_decrement_and_test is actually in use in libc, and for that the
i486 definition's semantics make sense.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2003-03-22 23:41 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-03-21 20:39 [PATCH] atomic.h fixes + testcase, misc fixes Jakub Jelinek
2003-03-22  0:11 ` Roland McGrath
2003-03-22  0:56   ` Jakub Jelinek
2003-03-22 23:52 ` Roland McGrath

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).