Re: Consistently use page_shift in sysdeps/unix/sysv/linux/mmap64.c

["Joseph S. Myers" <joseph at codesourcery dot com>]

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