On 31.08.2023 22:06, Shawn Anastasio wrote:
> On 8/29/23 8:59 AM, Jan Beulich wrote:
>> On 23.08.2023 22:07, Shawn Anastasio wrote:
>>> +{
>>> \
>>> + unsigned long old;
>>> \
>>> + unsigned int *p = (unsigned int *)_p;
>>> \
>>> + asm volatile (
>>> \
>>> + prefix
>>> \
>>> +"1: lwarx %0,0,%3,0\n"
>>> \
>>> + #op "%I2 %0,%0,%2\n"
>>> \
>>> + "stwcx. %0,0,%3\n"
>>> \
>>> + "bne- 1b\n"
>>> \
>>> + : "=&r" (old), "+m" (*p)
>>> \
>>> + : "rK" (mask), "r" (p)
>>> \
>>> + : "cc", "memory");
>>> \
>>
>> The asm() body wants indenting by another four blanks (more instances below).
>>
>
> If I were to match the style used in the previous patch's atomic.h, the
> body should be indented to line up with the opening ( of the asm
> statement, right?
Depends on whether the ( is to remain the last token on its line. If so, ...
> I'll go ahead and do that for consistency's sake
> unless you think it would be better to just leave it as-is with an extra
> 4 spaces of indentation.
... this style of indentation would want using. Either is fine, in case you
have a specific preference.
>>> +/* Based on linux/include/asm-generic/bitops/ffz.h */
>>> +/*
>>> + * ffz - find first zero in word.
>>> + * @word: The word to search
>>> + *
>>> + * Undefined if no zero exists, so code should check against ~0UL first.
>>> + */
>>> +#define ffz(x) __ffs(~(x))
>>> +
>>> +/**
>>> + * hweightN - returns the hamming weight of a N-bit word
>>> + * @x: the word to weigh
>>> + *
>>> + * The Hamming Weight of a number is the total number of bits set in it.
>>> + */
>>> +#define hweight64(x) generic_hweight64(x)
>>> +#define hweight32(x) generic_hweight32(x)
>>> +#define hweight16(x) generic_hweight16(x)
>>> +#define hweight8(x) generic_hweight8(x)
>>
>> Not using popcnt{b,w,d}, e.g. via a compiler builtin?
>>
>
> Excellent point. It looks like gcc's __builtin_popcount* family of
> builtins will do what we want here. I suppose the other architectures in
> Xen don't do this because they target toolchains old enough to not have
> these builtins?
Well, on x86 a patch introducing its use is actually pending ("x86: use
POPCNT for hweight<N>() when available").
>>> + tmp = (*p) & (~0UL >> (BITS_PER_LONG - size));
>>> + if (tmp == 0UL) /* Are any bits set? */
>>> + return result + size; /* Nope. */
>>> +found:
>>
>> Labels indented by at least one blank please. (More elsewhere.)
>
> I wasn't aware of this style convention -- so a single blank before the
> label would be correct?
Yes. (There are cases where deeper indentation is neater, but this isn't
one of them.)
Jan
