Re: [PATCH] maint: Use logical rather than bitwise operators on bools

[Jim Meyering]

Paul Eggert wrote:

> Pádraig Brady <address@hidden> writes:
>
>> As well as being distracting, bitwise operators are
>> not short circuiting and brittle with C89 where
>> bool can be an int, i.e. > 1.
>
> I don't find them distracting; on the contrary, I find them a helpful
> reminder that the expressions are boolean (in the traditional
> mathematical sense) and so I don't need to worry about the
> complexities of short-circuit evaluation.
>
> Also, the conditional branches generated by short-circuit operators
> can make code harder to debug because GDB starts jumping all over the
> place when I single step.
>
> Furthermore, bitwise operators often generate shorter and faster code,
> particularly on modern processors where conditional branches are far
> more expensive than logical operations.  For example, consider these
> functions:
>
> bool F (bool a, bool b, bool c, bool d) { return a && b && c && d; }
> bool G (bool a, bool b, bool c, bool d) { return a & b & c & d; }
>
> For F, gcc -O2 (x86) currently generates 3 conditional branches
> (ouch!); for G it generates straightline code (good with pipelining).
> F is also quite a bit bigger than G (20 instructions vs 12; and 49
> bytes versus 30), and this size increase bloats the code and causes
> more instruction cache misses.
>
> Bitwise operators are equivalent in behavior to short-circuit
> operations on boolean expressions (i.e., expressions involving just 0
> and 1) that do not have side effects or conditionally-undefined
> behavior. This is true regardless of whether the boolean type is
> implemented via 'int'.  Let's just leave them alone.

Hi Paul,

I was thinking of risk (not code size) when I ok'd his patch,
with this change still fresh in my memory:

    commit 3db7c2c03c4c6daf358925d1c585fcbb478fa25a
    Author: Jim Meyering <address@hidden>
    Date:   Thu Aug 20 10:36:30 2009 +0200

        install: avoid a portability bug when compiling with non-gcc

        * src/install.c (extra_mode): Be careful to return only a 0 or 1
        value, since this is a "bool" function, and there are still some
        compilers for which this is necessary.  Without this change,
        Bernhard Voelker reported that the Forte Developer 7 C 5.4 2002/03/09
        compiler would produce a malfunctioning "install" binary.  Details in
        http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/17710/focus=17760

Considering your arguments, I wonder if it's worthwhile to see
how many of those changes are in parts of the code where decreased
code size and fewer branches might make a difference.  Or go one
better...

Now that I think more about it, I really dislike pessimizing our code
to accommodate C89 (20 years old!) or any compiler that doesn't have
sufficient support for "bool", especially now that we require support
for some C99 features.

Pádraig, what would you think of my backing out your change, if I added
an (overridable) configure-time test to require working "bool"?