All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benny Halevy <bhalevy-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org>
To: Alexander van Heukelum
	<heukelum-hWlb6USbxJRiLUuM0BA3LQ@public.gmane.org>
Cc: Andrew Morton
	<akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>,
	linux-arch <linux-arch-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>,
	Andi Kleen <andi-Vw/NltI1exuRpAAqCnN02g@public.gmane.org>,
	LKML <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [0/3] Improve generic fls64 for 64-bit machines
Date: Thu, 03 Apr 2008 20:19:59 +0300	[thread overview]
Message-ID: <47F511BF.8090506@panasas.com> (raw)
In-Reply-To: <20080315172913.GA21648-hWlb6USbxJRiLUuM0BA3LQ@public.gmane.org>

On Mar. 15, 2008, 19:29 +0200, Alexander van Heukelum <heukelum-hWlb6USbxJRiLUuM0BA3LQ@public.gmane.org> wrote:
> This series of patches:
> 
> [1/3] adds __fls.h to asm-generic
> [2/3] modifies asm-*/bitops.h for 64-bit archs to implement __fls
> [3/3] modifies asm-generic/fls64.h to make use of __fls

I strongly support this.

I wish we'd also have a consistent naming convention for all
the bitops functions so it will be clearer what data type the
function is working on and is the result 0 or 1 based.

It seems like what we currently have is:

name	type	first bit#
----	----	----------
ffs	int	1
fls	int	1
__ffs	ulong	0
__fls	ulong	0	# in your proposal
ffz	ulong	0
fls64	__u64	1

so it seems like
- ffz is misnamed and is rather confusing.
  Apprently is should be renamed to __ffz.

- (new) ffz(x) can be defined to ffs(~(x))

- It'd be nice to have ffs64, and maybe ffz64.

Benny

> 
> I have compiled i386 and x86_64, and they generate the same code as
> before the change. The changes to the other archs are a best effort.
> Please comment.
> 
> If this patch series is accepted, it will make one tiny bit of
> the x86-unification a tiny bit cleaner. The patches are against
> Linus' current tree.
> 
> Andrew, if no concensus can be reached that this is a bad patch
> series, would you be willing to add this to your tree?
> 
> Greetings,
> 	Alexander
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

WARNING: multiple messages have this Message-ID (diff)
From: Benny Halevy <bhalevy@panasas.com>
To: Alexander van Heukelum <heukelum@mailshack.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-arch <linux-arch@vger.kernel.org>,
	Ingo Molnar <mingo@elte.hu>, Andi Kleen <andi@firstfloor.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [0/3] Improve generic fls64 for 64-bit machines
Date: Thu, 03 Apr 2008 20:19:59 +0300	[thread overview]
Message-ID: <47F511BF.8090506@panasas.com> (raw)
Message-ID: <20080403171959.7pzZhl-N0X9Oi-EeieU3q_u5fMbX8rvjglTdqw0t2Oo@z> (raw)
In-Reply-To: <20080315172913.GA21648@mailshack.com>

On Mar. 15, 2008, 19:29 +0200, Alexander van Heukelum <heukelum@mailshack.com> wrote:
> This series of patches:
> 
> [1/3] adds __fls.h to asm-generic
> [2/3] modifies asm-*/bitops.h for 64-bit archs to implement __fls
> [3/3] modifies asm-generic/fls64.h to make use of __fls

I strongly support this.

I wish we'd also have a consistent naming convention for all
the bitops functions so it will be clearer what data type the
function is working on and is the result 0 or 1 based.

It seems like what we currently have is:

name	type	first bit#
----	----	----------
ffs	int	1
fls	int	1
__ffs	ulong	0
__fls	ulong	0	# in your proposal
ffz	ulong	0
fls64	__u64	1

so it seems like
- ffz is misnamed and is rather confusing.
  Apprently is should be renamed to __ffz.

- (new) ffz(x) can be defined to ffs(~(x))

- It'd be nice to have ffs64, and maybe ffz64.

Benny

> 
> I have compiled i386 and x86_64, and they generate the same code as
> before the change. The changes to the other archs are a best effort.
> Please comment.
> 
> If this patch series is accepted, it will make one tiny bit of
> the x86-unification a tiny bit cleaner. The patches are against
> Linus' current tree.
> 
> Andrew, if no concensus can be reached that this is a bad patch
> series, would you be willing to add this to your tree?
> 
> Greetings,
> 	Alexander
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


  parent reply	other threads:[~2008-04-03 17:19 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       ` [PATCH] x86: fix description of __fls(): __fls(0) is undefined Alexander van Heukelum
2008-07-18 12:33         ` 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 [this message]
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=47F511BF.8090506@panasas.com \
    --to=bhalevy-c4p08nqkorlbdgjk7y7tuq@public.gmane.org \
    --cc=akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org \
    --cc=andi-Vw/NltI1exuRpAAqCnN02g@public.gmane.org \
    --cc=heukelum-hWlb6USbxJRiLUuM0BA3LQ@public.gmane.org \
    --cc=linux-arch-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mingo-X9Un+BFzKDI@public.gmane.org \
    /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.