This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: [PATCH] Add BeOS support for libstdc++-v3





On Tue, 21 Nov 2000, Richard Henderson wrote:

> > | + {
> > | +   unsigned long retval;
> > | +   long int readval;
> > | +   __asm__ ("lock");
> > | +   retval = *p;
> > | +
> 
> This is completely bogus.
Yes, i'm aware.
I thought I fixed it in the patch I sent to the mailing list, I must have 
sent one revision too early or smoething.
 > 
> One, you don't know for certain what instruction is going to
> follow the lock.  Two, the lock prefix is only applicable to
> specific instructions and movl isn't one of them.
> 
> > | +   __asm__ __volatile__ ("lock; xchgl %3, %1; sete %0"
> > | +                         : "=q" (ret), "=m" (*p), "=a" (readval)
> > | +                         : "r" (newval), "m" (*p));
> 
> Three, this does not even pretend to implement an atomic swap.
> Since xchgl does not set flags, this puts garbage into your
> return value.  In fact, since "ret" is not even defined, I can't
> imagine that this even compiles.

It does compile.
But it's also nto what's really there anymore.
Once again, this was the other thing i fixed in the revision of the BeOS 
patch I meant to send.
Luckily, these were teh only two fixes I ahd made.

 > 
> Four, this is hardly BeOS specific.  If code like this is to
> be included at all (instead of using atomicity.h), then it 
> should be ifdef __i386__ and written like so:
> 

Yup, I know.

>   inline unsigned long _Atomic_swap(unsigned long *__p, unsigned long q)
>   {
>     __asm__ __volatile__ ("lock; xchgl %0,%1"
> 			  : "=r"(q), "=m"(*__p)
> 			  : "m"(*__p), "0"(q));
>     return q;
>   }

This closely resembles what I have now.


> 
> 
> 
> r~
> 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]