From: Alexander van Heukelum <heukelum@mailshack.com>
To: "Ricardo M. Correia" <Ricardo.M.Correia@Sun.COM>,
Ingo Molnar <mingo@elte.hu>
Cc: Andrew Morton <akpm@linux-foundation.org>,
linux-arch <linux-arch@vger.kernel.org>,
Andi Kleen <andi@firstfloor.org>,
LKML <linux-kernel@vger.kernel.org>,
heukelum@fastmail.fm
Subject: [PATCH] x86: fix description of __fls(): __fls(0) is undefined
Date: Sat, 5 Jul 2008 19:53:46 +0200 [thread overview]
Message-ID: <20080705175345.GA7698@mailshack.com> (raw)
In-Reply-To: <1215276997.7167.12.camel@localhost>
Ricardo M. Correia spotted that the use of __fls() in fls64() did
not seem to make sense. In fact fls64()'s implementation is fine,
but the description of __fls() was wrong. Fix that.
Reported-by: "Ricardo M. Correia" <Ricardo.M.Correia@Sun.COM>
Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm>
---
On Sat, Jul 05, 2008 at 05:56:37PM +0100, Ricardo M. Correia wrote:
> Hi,
>
> I have a question about fls64() which I hope you or someone else could
> clarify, please see below.
>
> On Sáb, 2008-03-15 at 18:32 +0100, Alexander van Heukelum wrote:
> > +#elif BITS_PER_LONG == 64
> > +static inline int fls64(__u64 x)
> > +{
> > + if (x == 0)
> > + return 0;
> > + return __fls(x) + 1;
> > +}
>
> It seems fls64() is implemented on top of __fls(), however the __fls()
> implementation on the x86-64 architecture states that the result is
> undefined if the argument does not have any zero bits.
You have found a bug. It's not in fls64, though, but a copy/paste
one in the comment preceding __fls(). __fls() gives an undefined
result if there are no _set_ bits: only __fls(0) gives an undefined
result.
The inconsistency is well-spotted, though, thanks.
Patch is against current -tip.
Greetings,
Alexander
> So if I understand correctly, the statement "fls64(~0ULL)" would return
> an undefined result on x64-64 instead of 64 as one would expect.
>
> Wouldn't it make sense to check for ~0ULL in fls64()?
>
> Thanks,
> Ricardo
---
diff --git a/include/asm-x86/bitops.h b/include/asm-x86/bitops.h
index 96b1829..cfb2b64 100644
--- a/include/asm-x86/bitops.h
+++ b/include/asm-x86/bitops.h
@@ -356,7 +356,7 @@ static inline unsigned long ffz(unsigned long word)
* __fls: find last set bit in word
* @word: The word to search
*
- * Undefined if no zero exists, so code should check against ~0UL first.
+ * Undefined if no set bit exists, so code should check against 0 first.
*/
static inline unsigned long __fls(unsigned long word)
{
next prev parent reply other threads:[~2008-07-05 17:53 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-03-15 17:29 [0/3] Improve generic fls64 for 64-bit machines Alexander van Heukelum
2008-03-15 17:29 ` Alexander van Heukelum
[not found] ` <20080315172913.GA21648-hWlb6USbxJRiLUuM0BA3LQ@public.gmane.org>
2008-03-15 17:30 ` [1/3] Introduce a generic __fls implementation Alexander van Heukelum
2008-03-15 17:30 ` Alexander van Heukelum
2008-03-15 17:31 ` [2/3] Implement __fls on all 64-bit archs Alexander van Heukelum
2008-03-15 17:31 ` Alexander van Heukelum
2008-03-15 17:32 ` [3/3] Use __fls for fls64 on " Alexander van Heukelum
2008-03-15 17:32 ` Alexander van Heukelum
2008-07-05 16:56 ` Ricardo M. Correia
2008-07-05 17:53 ` Alexander van Heukelum [this message]
2008-07-18 12:33 ` [PATCH] x86: fix description of __fls(): __fls(0) is undefined Ingo Molnar
2008-03-21 13:10 ` [0/3] Improve generic fls64 for 64-bit machines Ingo Molnar
2008-03-21 13:10 ` Ingo Molnar
2008-04-03 17:19 ` Benny Halevy
2008-04-03 17:19 ` Benny Halevy
[not found] ` <47F511BF.8090506-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org>
2008-04-04 14:22 ` Alexander van Heukelum
2008-04-04 14:22 ` Alexander van Heukelum
2008-04-06 15:03 ` Benny Halevy
[not found] ` <47F8E64C.9030104-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org>
2008-04-06 19:10 ` Alexander van Heukelum
2008-04-06 19:10 ` Alexander van Heukelum
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20080705175345.GA7698@mailshack.com \
--to=heukelum@mailshack.com \
--cc=Ricardo.M.Correia@Sun.COM \
--cc=akpm@linux-foundation.org \
--cc=andi@firstfloor.org \
--cc=heukelum@fastmail.fm \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.