Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 12 May 2011 16:49:32 +0200
From:      Artem Belevich <art@freebsd.org>
To:        Attilio Rao <attilio@freebsd.org>
Cc:        svn-src-projects@freebsd.org, Oleksandr Tymoshenko <gonzo@freebsd.org>, src-committers@freebsd.org, Warner Losh <imp@freebsd.org>, Bruce Evans <brde@optusnet.com.au>
Subject:   Re: svn commit: r221614 - projects/largeSMP/sys/powerpc/include
Message-ID:  <BANLkTinaWDcaiZiB3G5Szoaho1jVSeniMA@mail.gmail.com>
In-Reply-To: <BANLkTik17r-XampEdO%2BsQ7aMOL_SDyhG=g@mail.gmail.com>
References:  <201105080039.p480doiZ021493@svn.freebsd.org> <BANLkTi=e7GtBM-PTq9yJHSLRoaOWh62AeA@mail.gmail.com> <BANLkTiktwEvRktZrGOqKKB2iSB99a3Jw=g@mail.gmail.com> <BANLkTik17r-XampEdO%2BsQ7aMOL_SDyhG=g@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, May 12, 2011 at 3:22 PM, Attilio Rao <attilio@freebsd.org> wrote:
> 2011/5/12 Artem Belevich <art@freebsd.org>:
>> On Thu, May 12, 2011 at 5:15 AM, Attilio Rao <attilio@freebsd.org> wrote=
:
>>> 2011/5/8 Attilio Rao <attilio@freebsd.org>:
>>>> Author: attilio
>>>> Date: Sun May =A08 00:39:49 2011
>>>> New Revision: 221614
>>>> URL: http://svn.freebsd.org/changeset/base/221614
>>>>
>>>> Log:
>>>> =A0All architectures define the size-bounded types (uint32_t, uint64_t=
, etc.)
>>>> =A0starting from base C types (int, long, etc).
>>>
>>> mips seems having the same issue, so here is my patch:
>>> http://www.freebsd.org/~attilio/largeSMP/mips-atomic.diff
>>
>> I see at least one problem. N32 ABI is ILP32 even though it can use
>> 64-bit registers for "long long".
>
> I'm sorry but this _longlong is non standard for our atomic scheme,
> nothing in the kernel uses it and in general I'd say to not add them
> if not strictly necessary.
> Said that, in the -CURRENT code it doesn't seem present, or I'm missing i=
t?

The key is that N32 ABI does use 64-bit registers and can perform
64-bit atomic ops.
Unfortunately, 64-bit type is long long or [u]int64_t. The point is
that it's not *long*.
>
>>> #if defined(__mips_n64) || defined(__mips_n32)
>>> static __inline void
>>>-atomic_set_64(__volatile uint64_t *p, uint64_t v)
>>>+atomic_set_long(__volatile u_long *p, u_long v)
>>
>> Using _long here would not match the assembly on N32.
>>
>> If you stick with your changes, then you should drop __mips_n32 from
>> the ifdef above.
>> You may also want to add atomic_*_longlong for n32. Actually, for o64 as=
 well.
>
> I'm not entirely sure in which way the above is different... or at
> least, I may have overlooked the _long support for __mips_n32... can
> you please elaborate a bit more?

The inline assembly for atomic_set_long above uses 64-bit load/store.
This is not going to work for 32-bit long in N32 ABI.

>
>> I think for mips sticking with atomic_*_<size> would be a better fit
>> considering ABI zoo we potentially have to deal with.
>
> Actually you still have them, it is just the dipendency that changes.

In the end you can do it either way, true. What I am trying to point
is that atomic ops are ususally involve fair amount of assembly code
where it's operand *size* is what matters, not what is the name of the
scalar of that size in particular ABI. If you use type to parametrize
atomic ops, then you run into the problem above -- you do need to
provide different implementation for the way given type is defined in
a given ABI.  In other words, type maps to assembly instructions in a
different way on different ABIs.

--Artem



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?BANLkTinaWDcaiZiB3G5Szoaho1jVSeniMA>