Compiler warnings from use of bitwise '|' and '&' for bool arguments

[perrydv]

I am getting compiler warnings like this:

warning: use of bitwise '|' with boolean operands [-Wbitwise-instead-of-logical] par_usage[i_par] = par_usage[i_par] | depend_x[j];

This is in local/optimize/get_par_usage.hpp at line 470 I believe. I think it wants '||' to be used instead.

I get a series of these from various places with '|' and '&'.

For single builds I could probably silence the warnings via choice of flags, but we call C++ compilers through R ("R CMD SHLIB"), and that is done on the fly with custom-generated code by the users of our package. Doing it this way lets us reliably get good C++ management on different operating systems, but it makes it hard and undesirable to take charge of compiler flags to silence warnings. Would it make sense to use '||' and '&&' in your source code? If so I can make a PR for you if you want. Thanks.

FWIW, I'm getting these warnings with clang++ on macOS 13.3.1, but as noted the issue is not this specific compiler.

[bradbell]

I often use | instead of || because | does not require a branch and hence does not interfere with compiler pipelining; see Branches on https://en.wikipedia.org/wiki/Instruction_pipelining#Branches

Often a compiler will require parentheses to make sure that the expressions are grouped in the intended order, but that does not seem to be a problem in the case above. Perhaps it is because no branch is required to implement the || in the case above (branches are required if there is more than one || or &&).

What compiler and verison and flags are you using ?

[perrydv]

I see. I'm in over my head on compiler details, but is it really a tradeoff between | not requiring branching and || not requiring evaluation of the second argument if the first is true? Would the compiler prefer to have slightly more "correct" code ( || for logical arguments, right?) and then be the one to decide how to implement it? In your code, is bool_a | bool_b really equivalent to something like static_cast<int>(bool_a) | static_cast<int>(bool_b)? (I am trying to make sense of info here, here, and here about type handling of arithmetic operators like bitwise | and the implicit conversions that are done, but I am very likely not following it all.)

The compiler is: clang++ --version Apple clang version 14.0.3 (clang-1403.0.22.14.1) Target: x86_64-apple-darwin22.4.0

We don't control the compiler flags because they are generated by "R CMD install", R's script for building packages, including C++ compilation steps. We can see the lines it generates, and the clang++ line includes "-Wall". For this and other reasons, it would be nice for us to quiet the warnings, so I just wanted to ask about them. If you have your reasons and don't want to change your code, that's fine and we'll go from there, but just wanted to know.

[bradbell]

Looking at the file bin/run_cmake.sh https://github.com/coin-or/CppAD/blob/master/bin/run_cmake.sh you will find the following text:

#
# cppad_cxx_flags
# clang++ 14.05 is warnings on bitwise operations with logical operands.
# These are used for speed, but maybe they do not help much ?
if [ "$clang" == 'yes' ]
then
   cppad_cxx_flags="$cppad_cxx_flags -Wno-bitwise-instead-of-logical"
fi

Perhaps you can add this flag to your "R CMD install" ? https://stackoverflow.com/questions/10921153/c-compilation-flags-from-r

[perrydv]

Thanks for pointing to that. The "other reasons" I was referring to (aside from R CMD install) are that our package generates C++ and compiles it on the fly on each user's system, so we've generally avoided trying to tune compiler flags for systems we don't manage. But anyway we'll look into it. Thanks and I'll close this now.

[bradbell]

It is hard to predict what an arbitrary compiler might warn for. If there is a way for you to check the compiler, then I would suggest only adding -Wno-bitwise-instead-of-logical when the compiler is clang++.

In general, it is a good idea to bring up warnings like this and make a decision if we will suppress the warning or change the source code. As the comment in bin/run_cmake.sh suggests, perhaps we should try changing all the | and & logical operations to || and && and see if it makes a difference on the speed of the program.