This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Add BeOS support for libstdc++-v3
- To: Richard Henderson <rth at redhat dot com>
- Subject: Re: [PATCH] Add BeOS support for libstdc++-v3
- From: Daniel Berlin <dberlin at cygnus dot com>
- Date: Wed, 22 Nov 2000 09:36:33 -0800 (PST)
- cc: Gabriel Dos Reis <gdr at codesourcery dot com>, Daniel Berlin <dan at www dot cgsoftware dot com>, gcc-patches at gcc dot gnu dot org, libstdc++ at gcc dot gnu dot org
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~
>