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

[Warner Losh]

On May 12, 2011, at 4:05 PM, Attilio Rao wrote:

> 2011/5/12 Attilio Rao <attilio at freebsd.org>:
>> 2011/5/12 Attilio Rao <attilio at freebsd.org>:
>>> 2011/5/12 Artem Belevich <art at freebsd.org>:
>>>> 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.
>>> 
>>> That cames from my mips assembly ignorance, I apologize.
>>> I just gave a look at types.h and what it just I basically see is:
>>> - __mips_n32 is just the same as plain !__mips_n32 && !__mips_n64
>>> - __mips_n64 just addes the operations for 64 bits operands
>> 
>> Oh, yeah, I forgot this part.
>> - __mips_n32 should be able to crunch the _64 variants, while
>> probabilly the !__mips_n32 && !__mips_n64 doesn't
>> 
>>> Thus what I need to make sure is:
>>> - __mips_n32 (and !__mips_n32 && !__mips_n64) case work on 32 bits
>>> operand on long too
>>> - __mips_n64 works on 64 bits long operands
>> 
>> - That __mips_n32 also defines the _64 variants.
> 
> I spoke in person with Artem and in the end I just decided to make the
> smallest possible subset of changes to fix the _long on 32 bits and
> then "completed" (as some of them already exist today) the macro
> converting the arguments to u_int stuff:
> http://www.freebsd.org/~attilio/largeSMP/mips-atomic2.diff
> 
> I compiled several kernels for MIPS (with sparse configurations and
> they all compile well).
> 
> If you agree with this patch, tonight I'll commit to my tree and add
> the mips support for cpuset_t soon, so that it all can be tested.

This change looks good.

Warner