0

I modified some chip manufacturer example code to remove a bunch of what I thought were stupid boolean comparisons, such as:

  • if(var == TRUE) → if(var)
  • if(TRUE == var) → if(var)
  • if(var != TRUE) → if(!var)
  • if(FALSE == var) → if(!var)
  • if(TRUE == var1 || TRUE == var2) → if(var1 || var2)
  • if(func() != FALSE) → if(func())
  • if(func() == TRUE) → if(func())

Where:

#define TRUE                                   1
#define FALSE                                  0

I thought this would make the code more readable and maybe even allow the compiler to optimize a little better, but the compiled code size increased by 118 bytes.

Am I missing something? These should be logically equivalent, right?

endolith
  • 25,479
  • 34
  • 128
  • 192
  • Just a style thing. Note that `TRUE` and `FALSE` are not standard `bool` values. – Eugene Sh. Apr 16 '21 at 18:40
  • @EugeneSh. But the compiler doesn't agree – endolith Apr 16 '21 at 18:40
  • 4
    Show [mcve] then. – Eugene Sh. Apr 16 '21 at 18:41
  • You say "bools" and "boolean", but what is the actual data type of these variables and functions? Is it in fact C `_Bool` or is it something else? – John Bollinger Apr 16 '21 at 18:45
  • 2
    Also if you are using some "homebrew" `bool` rather than standard one, these won't be equivalent: `if(var == TRUE) → if(var)` - because `var` might have values other than `0` and `1` and then the right side will be true, while the left won't. – Eugene Sh. Apr 16 '21 at 18:46
  • @EugeneSh. Yes but I would expect that to only make the code smaller, not larger. – endolith Apr 16 '21 at 18:50
  • @endolith You can examine the generated code and see if it makes sense. Again, [mcve] is needed to avoid speculations. – Eugene Sh. Apr 16 '21 at 18:52
  • 1
    Here is some comparison when using proper `bool` type vs `int`: https://godbolt.org/z/Y5PGr8fY1 As you can see when using `bool` all the constructs are equivalent. When using `int` it is different – Eugene Sh. Apr 16 '21 at 19:01
  • 2
    @endolith Why would you expect comparing to one to be easier than comparing to zero? I would expect the opposite -- it's quite likely that there are "compare with zero" operations while comparing with one will probably use a "compare with a constant" operation that needs one supplied as a constant. – David Schwartz Apr 16 '21 at 19:01
  • @JohnBollinger Ohhhhh you're probably right. Some of them were `unsigned char` but I changed them to [`bit` type](https://www.keil.com/support/man/docs/c51/c51_le_bit.htm). I forgot about those changes, and thought I had only made the changes listed above. If I revert the type changes, the code size only decreases. So I thought `bit` type would be optimized better, but it wasn't. – endolith Apr 16 '21 at 19:18
  • @DavidSchwartz Because `var` will evaluate to True regardless of whether it's 1, 2, 3, etc. So the compiler could … optimize … something. I thought it would give more flexibility. I guess not. – endolith Apr 16 '21 at 19:20
  • @endolith, C does not have a built-in type named `bit`, so I don't know what that is, but if it has a storage size larger than `unsigned char` then that could explain the difference right there. Do note that name notwithstanding, the `bit` type cannot have a storage size smaller than `unsigned char`'s, and it is not implausible that it would be larger. – John Bollinger Apr 16 '21 at 19:25
  • @JohnBollinger I linked to what it is. It does actually have a storage size of 1 bit, but apparently this doesn't equate to smaller compiled code. I tried changing one variable at a time, and the compiled code size went up, down, and up again. – endolith Apr 16 '21 at 19:27
  • Yeah so this is a bad question. I should either edit it to be about the code style itself and not the compiled size, or edit it to be about the bit type optimization, or delete it. – endolith Apr 16 '21 at 19:39
  • "The simplest and best way to get dramatic improvements in efficiency is to look for all variables that will have only binary values (0 and not 0), and define them as type ‘bit’" https://www.cypress.com/file/126981/download – endolith Apr 29 '21 at 13:53

2 Answers2

3

Ah, this is from the top of my head, but:

8051 has an instruction to jump conditioned on a single bit in a register, JB and JNB. If your compiler knows which bit to check, it can use that.

Alternatively, you can check the value in the accumulator being 0 or not 0, JZ, JNZ. But for that to work you might need to put the value on the accumulator first, which might need clearing and moving to that, adding overhead.

If your compiler isn't very old-school, use the built-in but not very standard _Bool or similar "pseudotypes", for which your compiler will work as smart as it can (and probably do what you intended to do).

Marcus Müller
  • 34,677
  • 4
  • 53
  • 94
  • I am not sure how this answers the question; the compiler will most likely generate the same code for either idiom. It is not a way if causing the compiler to generate optimal branches. The question is about whether there is a good reason for the coding style. This does not answer that. – Clifford Apr 17 '21 at 07:34
1

Not only is it unnecessary, it is a bad idea. Only comparison to FALSE is safe, since any non-zero value is implicitly true, but comparing such a value to TRUE may fail.

You did the right thing. Although you may need to consider maintenance. If the vendor updates this code, you may have to re-apply all your changes. Often it is simpler to live with third-party code as-is; no such code is ever going to confirm to your specific local coding standards or style - probably in moor ways than just this. Live with it or don't use it; certainly do not copy it.

In your own code you'd do better to use stdbool.h and bool, true and false. But you should still avoid explicit tests since it is unnecessary.

What I would avoid is implicit conversions to bool however. So for example a non-null pointer test should be if( ptr != NULL) rather than the common idiom if( ptr ). The 'rule' being that the condition expression should be explicitly Boolean. In your case if var is Boolean then there is no need to test for equality to true/false. Conversely if var is not Boolean, then you should test using the Boolean expressions 0 == var/0 != var.

The advice is largely about code quality, robustness, clarity and maintainability. I doubt that it will have any impact on code generation in any reasonable compiler.

Clifford
  • 88,407
  • 13
  • 85
  • 165