lkml.org 
[lkml]   [2011]   [Aug]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [patch 03/36] Hexagon: Add bitops support
    Date
    On Wednesday 17 August 2011 11:35:00 Richard Kuo wrote:

    > +
    > +#ifdef CONFIG_HEXAGON_BITOPS
    > +#include <asm/bitops/ffz.h>
    > +#include <asm/bitops/fls.h>
    > +#include <asm/bitops/ffs.h>
    > +#include <asm/bitops/__ffs.h>
    > +#include <asm/bitops/__fls.h>
    > +#else
    > +#include <asm-generic/bitops/ffz.h>
    > +#include <asm-generic/bitops/fls.h>
    > +#include <asm-generic/bitops/ffs.h>
    > +#include <asm-generic/bitops/__ffs.h>
    > +#define __fls(x) (fls(x) - 1)
    > +#endif

    Is there a particular reason why this is configurable? Also, I see no reaons
    to split these up into lots of files. Just put all of them into one file.

    > +/*
    > + * __ffs - find first bit in word.
    > + * @word: The word to search
    > + *
    > + * Undefined if no bit exists, so code should check against 0 first.
    > + *
    > + * bits_per_long assumed to be 32
    > + * numbering starts at 0 I think (instead of 1 like ffs)
    > + */
    > +static inline unsigned long __ffs(unsigned long word)
    > +{
    > + int num;
    > +
    > + asm volatile("%0 = ct0(%1);\n"
    > + : "=&r" (num)
    > + : "r" (word));
    > +
    > + return num;
    > +}

    I think this should not be marked volatile: If the result is unused,
    there is no need to compute the value either. Same for __fls, ffs,
    ffz, and fls.

    > +/*
    > + * ffs - find first bit set
    > + * @x: the word to search
    > + *
    > + * This is defined the same way as
    > + * the libc and compiler builtin ffs routines, therefore
    > + * differs in spirit from the above ffz (man ffs).
    > + */
    > +static inline int ffs(int x)
    > +{

    The type is normally 'long', not 'int'. They are obviously the
    same length on 32 bit machines, but you might get bogus compiler
    warnings from this. Same for ffz and fls.

    Arnd


    \
     
     \ /
      Last update: 2011-08-17 22:43    [W:2.904 / U:0.280 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site