On 26/08/2024 11:30 am, Jan Beulich wrote:
> On 23.08.2024 01:06, Andrew Cooper wrote:
>> This will be used to simplify real logic in the following patch. Add compile
>> and boot time testing as with other bitops.
>>
>> Because the expression is so simple, implement it as a function-like macro
>> which is generic on the type of it's argument, rather than having multiple
>> variants.
>>
>> Testing function-like macros needs a minor adjustments to the infrastructure
>> in xen/self-tests.h to avoid bracketing the fn parameter. The utility of
>> this
>> outweighs the associated risks.
> We may need to mark these two places as deviated.
Perhaps, although it would want to be a project-wide deviation.
Eclair was green with this patch in place, so it's not blocking.
>
>> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
> Reviewed-by: Jan Beulich <jbeul...@suse.com>
> with one remark/request:
>
>> --- a/xen/common/bitops.c
>> +++ b/xen/common/bitops.c
>> @@ -84,8 +84,32 @@ static void __init test_fls(void)
>> CHECK(fls64, 0x8000000000000001ULL, 64);
>> }
>>
>> +static void __init test_multiple_bits_set(void)
>> +{
>> + /*
>> + * multiple_bits_set() is generic on the type of it's parameter, as the
>> + * internal expression is so simple.
>> + */
>> +
>> + CHECK(multiple_bits_set, 0, false);
>> + CHECK(multiple_bits_set, 1, false);
>> + CHECK(multiple_bits_set, 2, false);
>> + CHECK(multiple_bits_set, 3, true);
>> +
>> + CHECK(multiple_bits_set, 1 | (1UL << (BITS_PER_LONG - 1)), true);
> This is really the same as ...
>
>> +#if BITS_PER_LONG > 32
>> + CHECK(multiple_bits_set, 1 | (1UL << 32), true);
>> + CHECK(multiple_bits_set, 1 | (1UL << 63), true);
> ... this, at least as long as BITS_PER_LONG > 32 in practice means
> BITS_PER_LONG == 64. Perhaps not really worth keeping that line?
I suppose not. I'll drop it.
However, It occurs to me that I do need a test of 0x8000000000000001ULL
mostly for 32bit builds to check that even when the argument is split,
the answer is still accurate.
~Andrew
