From owner-svn-src-all@freebsd.org Sat May 11 10:52:56 2019 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id C40D9159835D; Sat, 11 May 2019 10:52:56 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail105.syd.optusnet.com.au (mail105.syd.optusnet.com.au [211.29.132.249]) by mx1.freebsd.org (Postfix) with ESMTP id 34F3484CF9; Sat, 11 May 2019 10:52:55 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from [192.168.0.102] (c110-21-101-228.carlnfd1.nsw.optusnet.com.au [110.21.101.228]) by mail105.syd.optusnet.com.au (Postfix) with ESMTPS id 1B2D82ACDFF; Sat, 11 May 2019 20:52:46 +1000 (AEST) Date: Sat, 11 May 2019 20:52:44 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Doug Moore cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r347484 - head/sys/kern In-Reply-To: <201905110909.x4B99A5L006389@repo.freebsd.org> Message-ID: <20190511202741.H1251@besplex.bde.org> References: <201905110909.x4B99A5L006389@repo.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=P6RKvmIu c=1 sm=1 tr=0 cx=a_idp_d a=PalzARQSbocsUSjMRkwAPg==:117 a=PalzARQSbocsUSjMRkwAPg==:17 a=jpOVt7BSZ2e4Z31A5e1TngXxSK0=:19 a=kj9zAlcOel0A:10 a=nAMujBBYrrxrG9Oy4M0A:9 a=CjuIK1q_8ugA:10 X-Rspamd-Queue-Id: 34F3484CF9 X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.96 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-0.997,0]; NEURAL_HAM_SHORT(-0.96)[-0.962,0]; REPLY(-4.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0] X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 11 May 2019 10:52:56 -0000 On Sat, 11 May 2019, Doug Moore wrote: > Log: > When bitpos can't be implemented with an inline ffs* instruction, > change the binary search so that it does not depend on a single bit > only being set in the bitmask. Use bitpos more generally, and avoid > some clearing of bits to accommodate its current behavior. Inline ffs*() is badly implemented in general (it is mostly not implemented in the kernel). This makes it almost useless here, and (non-inline) ffs*() (not used here) often slower than direct methods. > Modified: head/sys/kern/subr_blist.c > ============================================================================== > --- head/sys/kern/subr_blist.c Sat May 11 04:18:06 2019 (r347483) > +++ head/sys/kern/subr_blist.c Sat May 11 09:09:10 2019 (r347484) > ... > +static inline int > +bitpos(u_daddr_t mask) > +{ > + > switch (sizeof(mask)) { > #ifdef HAVE_INLINE_FFSLL > case sizeof(long long): > return (ffsll(mask) - 1); > #endif This uses the long long abomination. Only amd64 has inline ffsll(). > +#ifdef HAVE_INLINE_FFS > + case sizeof(int): > + return (ffs(mask) - 1); > +#endif This is unreachable, since sizeof(int) is 4 on all supported arches, and sizeof(mask) is 8 on all arches. > default: > - lo = 0; > - hi = BLIST_BMAP_RADIX; > - while (lo + 1 < hi) { > - mid = (lo + hi) >> 1; > - if ((mask >> mid) != 0) > - lo = mid; > - else > - hi = mid; > - } > - return (lo); > + return (generic_bitpos(mask)); > } > } So the default cause is used except on amd64. I think the direct code for it is not as slow as the generic (libkern) ffs*(), except the generic ffs*() is simpler so the compiler might inline it internals to __builtin_ffs*(). Not that -ffreestanding prevents inlining extern ffs*(). Since 64/8 is 2 times 4 and sizeof(int) >= 4 in POSIX, it is trivial to implement ffs64() using 2 inline ffs()'s, and that is exactly what clang does on i386 to implement __builtin_ffsl(). Unfortunately, most arches also don't implement inline ffs(). Only amd64, i386 and sparc64 implement it. subr_blist.c is also complilable outside of the kernel. I don't like that. The kernel macros HAVE_INLINE_* don't exist outside of the kernel, but ffs*() would work better outside of the kernel if it is used blindly since inlining of it isn't prevented by -ffreestanding. Bruce