svn commit: r221614 - projects/largeSMP/sys/powerpc/include

[Artem Belevich]

On Thu, May 12, 2011 at 3:22 PM, Attilio Rao <attilio at freebsd.org> wrote:
> 2011/5/12 Artem Belevich <art at freebsd.org>:
>> On Thu, May 12, 2011 at 5:15 AM, Attilio Rao <attilio at freebsd.org> wrote:
>>> 2011/5/8 Attilio Rao <attilio at freebsd.org>:
>>>> Author: attilio
>>>> Date: Sun May  8 00:39:49 2011
>>>> New Revision: 221614
>>>> URL: http://svn.freebsd.org/changeset/base/221614
>>>>
>>>> Log:
>>>>  All architectures define the size-bounded types (uint32_t, uint64_t, etc.)
>>>>  starting 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 it?

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