[Skiboot] [PATCH 1/5] simplify GET/SETFIELD() use, add MASK_TO_LSH()

[Dan Streetman]

On Tue, Feb 17, 2015 at 3:44 PM, Benjamin Herrenschmidt
<benh at kernel.crashing.org> wrote:
> On Tue, 2015-02-17 at 15:38 -0500, Dan Streetman wrote:
>> Currently, the GETFIELD() and SETFIELD() macros require the user to define
>> both the _MASK and the _LSH of the field. However, the shift of the field
>> is trivially determinable and there is no reason it needs to be specified
>> by the user.
>>
>> Add MASK_TO_LSH() macro, which determines the shift of a given mask.
>>
>> Change GETFIELD() and SETFIELD() to use MASK_TO_LSH() to determine the
>> shift of the specified mask, instead of requiring it to be specified by
>> the user.
>
> This is quite nice, did you check gcc was resolving it properly at
> compile time ?

Yep, gcc compiles it fine and I booted the resulting skiboot.lid fine
also.  I'm pretty sure that all gcc versions implement
__builtin_ffsl()...probably any other compiler would too, although I
assume we really only plan on gcc.

>
> We could get rid of a whole pile of our _LSH definitions all over the
> place...

yep, next patch in the set does that, and removes a lot of the _MASK suffixes.

>
>> Signed-off-by: Dan Streetman <ddstreet at ieee.org>
>> ---
>>  include/bitutils.h | 11 ++++++-----
>>  1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/bitutils.h b/include/bitutils.h
>> index 7e5b6eb..baa752b 100644
>> --- a/include/bitutils.h
>> +++ b/include/bitutils.h
>> @@ -36,15 +36,16 @@
>>   * PPC bitmask field manipulation
>>   */
>>
>> +/* Find left shift from first set bit in mask */
>> +#define MASK_TO_LSH(m)               (__builtin_ffsl(m) - 1)
>> +
>>  /* Extract field fname from val */
>> -#define GETFIELD(fname, val)                 \
>> -     (((val) & fname##_MASK) >> fname##_LSH)
>> +#define GETFIELD(m, v)               (((v) & (m)) >> MASK_TO_LSH(m))
>>
>>  /* Set field fname of oval to fval
>>   * NOTE: oval isn't modified, the combined result is returned
>>   */
>> -#define SETFIELD(fname, oval, fval)                  \
>> -     (((oval) & ~fname##_MASK) | \
>> -      ((((typeof(oval))(fval)) << fname##_LSH) & fname##_MASK))
>> +#define SETFIELD(m, v, val)                          \
>> +     (((v) & ~(m)) | ((((typeof(v))(val)) << MASK_TO_LSH(m)) & (m)))
>>
>>  #endif /* __BITUTILS_H */
>
>
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot