Re: svn commit: r227812 - head/lib/libc/string

[ Available lists | Index of svn-src-head | Month of Nov 2011 | Week of 22 Nov 2011 | Raw email | View thread | Wrap long lines | Reply | Tag ]
From
David Schultz <das@FreeBSD.ORG>
Date
22 Nov 2011 15:33:34
Subject
Re: svn commit: r227812 - head/lib/libc/string
Message-ID
20111122153332.GA20145@zim.MIT.EDU


[ Hide this part ]
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.)


Elapsed time: 0.181 seconds