Date: Tue, 22 Nov 2011 10:33:32 -0500 From: David Schultz <das@FreeBSD.ORG> To: Eitan Adler <eadler@FreeBSD.ORG> Cc: svn-src-head@FreeBSD.ORG, svn-src-all@FreeBSD.ORG, src-committers@FreeBSD.ORG Subject: Re: svn commit: r227812 - head/lib/libc/string Message-ID: <20111122153332.GA20145@zim.MIT.EDU> In-Reply-To: <201111220250.pAM2oPWC070856@svn.freebsd.org> References: <201111220250.pAM2oPWC070856@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Nov 22, 2011, Eitan Adler wrote: > + /* use a bitwise or to avoid an additional branch instruction */ > + if ((s1 == s2) | (n == 0)) > + return (0); I think there are three issues with this. First, the comment suggesting that using '|' instead of '||' isn't correct; any reasonable compiler knows how to optimize side-effect-free expressions like these. (The reverse transformation, from the arithmetic expression to the boolean one, is actually harder for the compiler in general.) Second, the overwhelming precedent in FreeBSD is to use boolean operators to combine boolean expressions, so you might try to get some consensus on the issue before you go around replacing them with bitwise operators. I for one don't find the bitwise operators clearer, but I don't speak for everyone else. Third, it's not clear that checking whether s1 == s2 is even an optimization. Most programs simply aren't going to pass identical pointers as arguments to strcmp(), so for the overwhelming majority of cases, this is just a wasted test and a wasted slot in a branch predict table. (FWIW, I doubt that a realistic benchmark would demonstrate any measurable difference.)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111122153332.GA20145>