Re: [Various] libc/1161: Unable to get nscd to work on alpha

[Geoff Keating <geoffk@ozemail.com.au>]

> Mailing-List: contact libc-alpha-help@sourceware.cygnus.com; run by ezmlm
> cc: Andreas Jaeger <aj@arthur.rhein-neckar.de>,
>         libc-alpha Mailinglist <libc-alpha@sourceware.cygnus.com>,
>         sba@srl.caltech.edu
> Date: Sat, 12 Jun 1999 21:01:35 -0400
> From: Horst von Brand <vonbrand@sleipnir.valparaiso.cl>
> 
> Thorsten Kukuk <kukuk@suse.de> said:
> > On Sat, Jun 12, Andreas Jaeger wrote:
> > > Is there anything we can do to fix nscd on alpha?  I fear this is a
> > > compiler bug:-(.  Richard, can you help?
> 
> > It's an older bug which I have already send twice to the egcs people
> > without getting an reaction.
> > 
> > egcs miscompiles the following lines from nscd/cache.c:
> > 
> >   /* Put the new entry in the first position.  */
> >   do
> >     newp->next = table->array[hash];
> >   while (! compare_and_swap ((volatile long int *) &table->array[hash],
> 
> Have you tried adding -fno-strict-aliases? Without it late egcs snapshots
> at least assume data can't be accessed through a pointer to a different
> type, unless it is a char pointer (this is what the standard says, and egcs
> follows). Bit me hard.

I notice that the ppc version of this has a bug; the assembler isn't
clobbering "memory".  It's always necessary for compare_and_swap to
have a clobber of "memory" somewhere, otherwise egcs may delay a store
until after the compare-and-swap.  I attach a patch.

The alpha version of this is different, in that it uses a "m"
constraint instead of taking the address like the ppc one does.
However, it _still_ needs a 'memory' clobber, because otherwise code
like:

{
  static long lock;
  static uint32_t lfsr;

  /* Acquire a lock, spinning until we get it.  */
  while (!compare_and_swap (&lock, 0, 1)) ;

  /* Change some memory location, protected by the lock.  */
  lfsr = lfsr >> 1 ^ ((lfsr & 1) ? 0x80003000 : 0);

  /* Release the lock.  */
  compare_and_swap(&lock, 1, 0);
}

may be rescheduled by gcc so that the assignment happens before the
acquisition of the lock, or after the release, or even some
combination of the two.

Then you don't need to use -fno-strict-aliases.

-- 
Geoffrey Keating <geoffk@ozemail.com.au>

===File ~/patches/glibc-50.diff=============================
1999-06-13  Geoff Keating  <geoffk@ozemail.com.au>

	* sysdeps/powerpc/atomicity.h (exchange_and_add): Remove `volatile';
	add `memory' clobber; optimise for constant `val'.
	(atomic_add): Likewise.
	(test_and_set): Remove `volatile'; add `memory' clobber; be more
	like the original `test_and_set'.
	(compare_and_swap): Remove `volatile'; add `memory' clobber;
	optimise for constant `oldval'.
	(always_swap): Remove `volatile'; add `memory' clobber.

--- atomicity.h.old	Sun Jun 13 15:56:33 1999
+++ atomicity.h	Sun Jun 13 15:45:21 1999
@@ -1,5 +1,5 @@
 /* Low-level functions for atomic operations.  PowerPC version.
-   Copyright (C) 1997, 1998 Free Software Foundation, Inc.
+   Copyright (C) 1997, 1998, 1999 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
@@ -33,12 +33,12 @@ __attribute__ ((unused))
 exchange_and_add (volatile uint32_t *mem, int val)
 {
   int tmp, result;
-  __asm__ __volatile__ ("\
+  __asm__ ("\
 0:	lwarx	%0,0,%2
-	add	%1,%3,%0
+	add%I3	%1,%0,%3
 	stwcx.	%1,0,%2
 	bne-	0b
-" : "=&r"(result), "=&r"(tmp) : "r" (mem), "r"(val) : "cr0");
+" : "=&b"(result), "=&r"(tmp) : "r" (mem), "Ir"(val) : "cr0", "memory");
   return result;
 }
 
@@ -47,12 +47,12 @@ __attribute__ ((unused))
 atomic_add (volatile uint32_t *mem, int val)
 {
   int tmp;
-  __asm__ __volatile__("\
+  __asm__ ("\
 0:	lwarx	%0,0,%1
-	add	%0,%2,%0
+	add%I2	%0,%0,%2
 	stwcx.	%0,0,%1
 	bne-	0b
-" : "=&r"(tmp) : "r" (mem), "r"(val) : "cr0");
+" : "=&b"(tmp) : "r" (mem), "Ir"(val) : "cr0", "memory");
 }
 
 static __ATOMICITY_INLINE int
@@ -60,16 +60,16 @@ __attribute__ ((unused))
 compare_and_swap (volatile long int *p, long int oldval, long int newval)
 {
   int result;
-  __asm__ __volatile__ ("\
+  __asm__ ("\
 0:	lwarx	%0,0,%1
-	xor.	%0,%0,%2
+	sub%I2c.	%0,%0,%2
 	cntlzw	%0,%0
 	bne-	1f
 	stwcx.	%3,0,%1
 	bne-	0b
-1:	srwi	%0,%0,5
-" : "=&r"(result) : "r"(p), "r"(oldval), "r"(newval) : "cr0");
-  return result;
+1:	
+" : "=&b"(result) : "r"(p), "Ir"(oldval), "r"(newval) : "cr0", "memory");
+  return result >> 5;
 }
 
 static __ATOMICITY_INLINE long int
@@ -77,28 +77,27 @@ __attribute__ ((unused))
 always_swap (volatile long int *p, long int newval)
 {
   long int result;
-  __asm__ __volatile__ ("\
+  __asm__ ("\
 0:	lwarx	%0,0,%1
 	stwcx.	%2,0,%1
 	bne-	0b
-" : "=&r"(result) : "r"(p), "r"(newval) : "cr0");
+" : "=&r"(result) : "r"(p), "r"(newval) : "cr0", "memory");
   return result;
 }
 
 static __ATOMICITY_INLINE int
 __attribute__ ((unused))
-test_and_set (volatile long int *p, long int oldval, long int newval)
+test_and_set (volatile long int *p, long int newval)
 {
   int result;
-  __asm__ __volatile__ ("\
+  __asm__ ("\
 0:	lwarx	%0,0,%1
-	xor.	%0,%0,%2
-	cntlzw	%0,%0
+	cmpwi	%0,0
 	bne-	1f
-	stwcx.	%3,0,%1
+	stwcx.	%2,0,%1
 	bne-	0b
-1:	srwi	%0,%0,5
-" : "=&r"(result) : "r"(p), "r"(oldval), "r"(newval) : "cr0");
+1:
+" : "=&r"(result) : "r"(p), "r"(newval) : "cr0", "memory");
   return result;
 }
 
============================================================