5

In my code, I need to saturate a variable uint32_t var within a range [MIN, MAX]. MIN and MAX are defined as macros, so I can change these values easily inside a header file.

Header:

#define MIN    (0)
#define MAX    (1000)

Code:

if(var > MAX){
    var = MAX;
}else if(var < MIN){
    var = MIN;
}

When I now use compiler flag -Wtype-limits, I get a warning that the second check var < MIN is always false. This warning suggests that I can remove the second comparison.

This makes sense as long as I define #define MIN (0) but, when changing this to let's say #define MIN (10), then the second comparison is mandatory.

So my question: How can I tell the compiler, that MIN can be any value greater or equal zero?

phuclv
  • 37,963
  • 15
  • 156
  • 475
paul_schaefer
  • 397
  • 4
  • 16
  • by using #UNDEF symbol? and not to the compiler, but to the CPP, which generates the translation unit... – alinsoar Aug 25 '21 at 10:50

3 Answers3

5

The simplest solution is just use <= instead of <

if (var >= MAX) {
    var = MAX;
} else if (var <= MIN) {
    var = MIN;
}

But you should avoid macros if possible and just declare constants instead. Besides always use the U suffix when you work with unsigned values to avoid signed-unsigned incompatibility warnings. In fact it's quite interesting to see that if I use #define like that I get a warning in GCC but not Clang, and when I change to const and add the U suffix then I get no warning with GCC but one with Clang

phuclv
  • 37,963
  • 15
  • 156
  • 475
  • I also thought about using ```<=``` instead of ```<```. The only "disadvantage" is, that I reassign 0 to ```var``` even it is already zero. This "takes time" and i wanted to keep the interrupt handler as short as possible. Sure, the time lost won't be so much.... – paul_schaefer Aug 25 '21 at 11:22
  • @paul_schaefer there's no difference in that case with an optimizing compiler. Just check the compiler outputs. For example if MIN = 10 then it'll only assign when var < 10, and there's no check for zero if it's not necessary – phuclv Aug 25 '21 at 11:31
  • @paul_schaefer Gcc seems already smart enough not to generate the else if branch for `else if (unsigned_var <= 0) unsigned_var=0;`, clang can be helped with `var <= MIN && var!=MIN`. See https://godbolt.org/z/YehPWx64Y. – Petr Skocik Aug 25 '21 at 11:33
  • @phuclv: concerning the compiler example in your answer. In the output line 20 there is the comparison with the value 10 and in line 21 there is written the command ```ja```. And as I understood this means "jump if above" so the jump is only performed, if the value in eax is greater then 10. So if the value in eax is 10, then the jump is not performed an 10 is written back. So IMHO the writing is not skipped, or did I misunderstood anything? – paul_schaefer Aug 25 '21 at 11:56
  • @paul_schaefer you're right, probably that's a missed optimization of GCC. ICC can avoid that assignment when var = 10 or 1000 https://godbolt.org/z/nhcfqK846, but it also prefers branchless solutions so the result is the same for any values of var. Anyway this is micro-optimization and shouldn't be worried about unless you see it's really the hot spot. Always compile with PGO and profile first – phuclv Aug 25 '21 at 12:17
  • In C++ using `std::clamp` it's much cleaner https://godbolt.org/z/ozfo4hbGE without any unnecessary assignment – phuclv Aug 25 '21 at 12:57
3

This solves the issue:

int main(void) {
    unsigned int var = rand();

    if (var > MAX){
        var = MAX;
    }
#if MIN > 0
    else if (var < MIN) {
        var = MIN;
    }
#endif

    printf("token = %u\n", var);
}

Edit: reduce "dirtiness" by making it clear what the two cases are.

uint32_t saturate(uint32_t var)
{
#if MIN > 0
    if (var > MAX){
        var = MAX;
    }
    else if (var < MIN) {
        var = MIN;
    }
#else
    // Only need to do the upper bound
    if (var > MAX){
        var = MAX;
    }
#endif
    return var;
}
stark
  • 12,615
  • 3
  • 33
  • 50
0

How can I tell the Compiler, that MIN can be any value greater or equal zero?

You can't, you just told it that it is exactly zero.

This warning comes from the compiler's ability to evaluate integer constant expressions at compile time, so it sees var < 0 and spots that var is an unsigned type.

We can prevent that and get rid of the warning by using a variable instead, so it's no longer an integer constant expression:

const unsigned int MIN = 0;

Or alternatively, if you must have a macro for some reason, use a compound literal:

#define MIN    (const unsigned int){0}

Now of course don't write weird stuff like that without comments, so the correct production code would be for example:

// MIN is using compound literal to block unsigned vs 0 comparison warnings in gcc 1.2.3
#define MIN (const unsigned int){0}
Lundin
  • 195,001
  • 40
  • 254
  • 396
  • IIRC, without the `static` specifier, the `const unsigned int MIN = 0;` line added in *a header* will break the "One Definition Rule", if that header is included by multiple source files. – Adrian Mole Aug 25 '21 at 11:11
  • @AdrianMole Yes, don't define variables in headers. If it needs to be in a header then do `extern const unsigned int MIN;` and put the definition in a .c file. The compound literal version is probably more convenient then. – Lundin Aug 25 '21 at 11:14