Consistently use page_shift in sysdeps/unix/sysv/linux/mmap64.c
Joseph S. Myers
joseph@codesourcery.com
Tue Jul 2 14:58:00 GMT 2013
On Tue, 2 Jul 2013, Steven Munroe wrote:
> 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.
__builtin_ffs would fall back to ffs, which is not namespace-clean - the
general issue I referred to. __builtin_ctz would fall back to a libgcc
function, which is safe, but arbitrarily different from other uses of
__ffs. Whatever the solution, it should be consistent rather than doing
something different just here.
--
Joseph S. Myers
joseph@codesourcery.com
More information about the Libc-alpha
mailing list