This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: Consistently use page_shift in sysdeps/unix/sysv/linux/mmap64.c
- From: Steven Munroe <munroesj at linux dot vnet dot ibm dot com>
- To: "Joseph S. Myers" <joseph at codesourcery dot com>
- Cc: Andreas Schwab <schwab at suse dot de>, Roland McGrath <roland at hack dot frob dot com>, libc-alpha at sourceware dot org
- Date: Tue, 02 Jul 2013 09:53:11 -0500
- Subject: Re: Consistently use page_shift in sysdeps/unix/sysv/linux/mmap64.c
- References: <Pine dot LNX dot 4 dot 64 dot 1306282000050 dot 15167 at digraph dot polyomino dot org dot uk> <20130628213457 dot 38E962C09C at topped-with-meat dot com> <Pine dot LNX dot 4 dot 64 dot 1307012238110 dot 13535 at digraph dot polyomino dot org dot uk> <mvmy59ppgcu dot fsf at hawking dot suse dot de> <Pine dot LNX dot 4 dot 64 dot 1307021249050 dot 18631 at digraph dot polyomino dot org dot uk>
- Reply-to: munroesj at us dot ibm dot com
On Tue, 2013-07-02 at 12:52 +0000, Joseph S. Myers wrote:
> On Tue, 2 Jul 2013, Andreas Schwab wrote:
>
> > "Joseph S. Myers" <joseph@codesourcery.com> writes:
> >
> > > + page_shift = __ffs (page_size) - 1;
> >
> > This has the effect of using a function call instead of the builtin ffs.
>
> True, but not in any way specific to this patch. Rather, this is just one
> instance of the general issue of the right way to call functions within
> glibc such that both (a) any applicable compiler optimizations are applied
> and (b) if a particular call does fall back to calling a function, that
> function call is namespace-clean. It doesn't make sense to do something
> special for one particular __ffs call; any fix would be something that
> makes all __ffs calls work right. And optimization isn't really
> significant for this particular code, executed (subject to races) at most
> once in a program execution. The point of using __ffs is to eliminate the
> possibility of a race causing incorrect execution, not to optimize things.
>
Why not use __builtin_ffs or __builtin_ctz here. Most systems now have
specific instructions for this and related operations and should be
inline.