pic18, xc8, v2.05, arithmetic overflow in constant expression

Thread Starter

bug13

Joined Feb 13, 2012
2,002
For the completeness, I just quickly tested this with a minimum project

Same environment as above, still got the same warning.

Code:
/*
* File:   main.c
* Author: james
*
* Created on 16 April 2019, 3:05 PM
*/


#include <xc.h>
#include <stdint.h>

void main(void) {
    SPBRG = (uint8_t) (416U & 0x00FF);
}
 

spinnaker

Joined Oct 29, 2009
7,830
No, you haven't found a bug. What you've found is the optimizing particularities of a 8-bit register C compiler with 16-bit integral promotions. A decision has been made over and over again to leave the warning at the current level because of sometimes very subtle embedded system bugs in code because of implicit type promotion in C. The easy solution(s) to this warning has been given on this thread a few times already.

Not sure I am buy buying that. I would if it were simply that he hard coded the setting of SPBRG .

But he also gets it when he gets the low byte of an unsigned integer variable.

unsigned a=416;
SPBRG = (unsigned char)(a & 0x00FF);

That is a common operation. It shouldn't cause a warning optimization or not.
 

MrChips

Joined Oct 2, 2009
34,829
I don't use XC8 and hence I have no clue what's that about.

Apparently, the compiler treats 416 as bigger than uint8_t whereas SPBRG has been declared as uint8_t.
The compilers with which I am familiar will simply pass the lower byte without complaining.
(To be honest, I have to verify this later today.)
 

nsaspook

Joined Aug 27, 2009
16,330
Not sure I am buy buying that. I would if it were simply that he hard coded the setting of SPBRG .

But he also gets it when he gets the low byte of an unsigned integer variable.

unsigned a=416;
SPBRG = (unsigned char)(a & 0x00FF);

That is a common operation. It shouldn't cause a warning optimization or not.
XC8 is annoyingly pedantic with its warnings, but it's not incorrect for the embedded domain where people sometimes use 8-bit register overflows to optimize time critical routines.
 

BobTPH

Joined Jun 5, 2013
11,533
I tried this:
It didn't compile:
It should not compile. In C there are lvalues and rvalues (for left and right). lvalules can be assigned to, rvalues cannot. A variable by itself is an lvalue. A variable casted is an rvalue, and hence cannot be assigned to. There is a way to cast on the left side of the expression. If you want to assign to the low byte of an int, for example:

*(unsigned char *)(&intvar) = x;

Will be accepted by the compiler (but will not likely help you out in getting rid of the error), This cast casts the address of the variable to be a pointer to a char, then defreferences that pointer to access a char.

Having worked on compilers professionally for over 30 years, including Microsoft C++ and Intel C++, I know a little bit about compilers. What is going on here is that the compiler is noticing the possibility of an overflow by looking only at the data types, before evaluating the expression, and hence the warning. Warnings are not errors and do not necessarily mean the program is incorrect, thus the compiler goes on to produce the correct output anyway. A warning is only there to inform the user that he *might* be doing something wrong, and should take a second look at the code to verify that it does what he wants. There is typically a way to turn off warnings, often with the granularity of the specific warning message.

Bob
 
Last edited:
xc8 2.05 optimization=0, error level 0 (default I think)

C:
#include <stdint.h>

#define REGVAL (416u & 0x00ffu)

void main(void) {
  uint8_t SPBRG;
  uint16_t s2=0;
   
   
  SPBRG=(uint8_t) (416 & 0x00ff );  // WARN
  SPBRG=(uint8_t) ( (unsigned int)(416 & 0x00ff) );  // WARN
  SPBRG=(uint8_t) ( (unsigned int)(416u & 0x00ffu) ); // WARN
  SPBRG=(uint8_t)REGVAL;  // WARN
  SPBRG=(uint8_t) (416 & 0x0f);  // WARN
  SPBRG=(uint8_t) ( s2= (416u & 0x00ffu ) );  // OK
  SPBRG=(uint8_t) 416u;  // WARN
  SPBRG=(uint8_t) (s2= 416u);  // OK
   
  return;
}
Because 416 can't fit into 8 bits, it is going to warn you [warning: (751) arithmetic overflow in constant expression], wherever you possibly try to force it into 8 bits. What the OP wants it to do is calculate (416 & 0x00ff) FIRST and then see that the result can fit into 8 bits and leave you alone. It does not do that and it is not going to let you typecast your way out of that - I am comfortable with such behavior.

If you want to put the result in a variable of an appropriate size and then typecast it to uint8_t, it is ok with that, but note that there is no warning if the typecast is going to change the value [SPBRG=(uint8_t) (s2= 416u); // OK]. In that case I definitely would want to see a warning of some kind!
 

nsaspook

Joined Aug 27, 2009
16,330
xc8 2.05 optimization=0, error level 0 (default I think)

C:
#include <stdint.h>

#define REGVAL (416u & 0x00ffu)

void main(void) {
  uint8_t SPBRG;
  uint16_t s2=0;
 
 
  SPBRG=(uint8_t) (416 & 0x00ff );  // WARN
  SPBRG=(uint8_t) ( (unsigned int)(416 & 0x00ff) );  // WARN
  SPBRG=(uint8_t) ( (unsigned int)(416u & 0x00ffu) ); // WARN
  SPBRG=(uint8_t)REGVAL;  // WARN
  SPBRG=(uint8_t) (416 & 0x0f);  // WARN
  SPBRG=(uint8_t) ( s2= (416u & 0x00ffu ) );  // OK
  SPBRG=(uint8_t) 416u;  // WARN
  SPBRG=(uint8_t) (s2= 416u);  // OK
 
  return;
}
Because 416 can't fit into 8 bits, it is going to warn you [warning: (751) arithmetic overflow in constant expression], wherever you possibly try to force it into 8 bits. What the OP wants it to do is calculate (416 & 0x00ff) FIRST and then see that the result can fit into 8 bits and leave you alone. It does not do that and it is not going to let you typecast your way out of that - I am comfortable with such behavior.

If you want to put the result in a variable of an appropriate size and then typecast it to uint8_t, it is ok with that, but note that there is no warning if the typecast is going to change the value [SPBRG=(uint8_t) (s2= 416u); // OK]. In that case I definitely would want to see a warning of some kind!
It doesn't warn in the [SPBRG=(uint8_t) (s2= 416u); // OK] case because there are not intermediate operand operations with possible side-effects from integral promotions.

https://wiki.sei.cmu.edu/confluence/display/c/INT02-C.+Understand+integer+conversion+rules
 

Ian Rogers

Joined Dec 12, 2012
1,136
Does this work

SPBRG=(uint8_t)416 & 0xff ;

As You should only need to cast the 16 bit to an 8 bit... BUT!! as everyone said... Why?

SPBRG=160;

is much easier....
 

djsfantasi

Joined Apr 11, 2010
9,237
Does this work

SPBRG=(uint8_t)416 & 0xff ;

As You should only need to cast the 16 bit to an 8 bit... BUT!! as everyone said... Why?

SPBRG=160;

is much easier....
I totally agree... but even I missed the point. (uint8_t)(416) SHOULD throw a warning, because that operation can’t be done. 416 is a 9 bit value.
 

nsaspook

Joined Aug 27, 2009
16,330
I would want to see a warning after SPBRG=(uint8_t) (s2= 416u); // OK but I would not expect it to be an integral promotion warning [section 5.6]

https://www.mouser.com/datasheet/2/268/52053B-477133.pdf

I would want to see a warning like (752) conversion to shorter data type. I say that, however, knowing that "you can't always get what you want"
This is an instance where the cast means, the programmer 'knows what they are doing', so a warning is meaningless if there are no side-effects.
 

MrChips

Joined Oct 2, 2009
34,829
It must be an XC8 thingy.

I can do these with no warnings on IAR Embedded Workbench and Atollic TrueSTUDIO.

SPBRG = 416 & 0xFF;
SPBRGH = 416 >> 8;
 
We know that doesn’t work. But
SPBRG = 216;​
does not generate a warning. So what’s that about? :eek:
When SPBRG is declared as uint8_t...
and SPBRG=416, you get a legitimate warning...
warning: implicit conversion from 'int' to 'uint8_t' (aka 'unsigned char') changes value from 416 to 160 [-Wconstant-conversion]
SPBRG=416;

and, as you noted, if SPBRG=216, you do not.
 
Yes. Did that.
Interesting. Using IDE 5.15 and xc8 2.05

C:
#include <xc.h>
#include <stdint.h>

void main(void) {
  uint8_t SPBRG,SPBRGH;
  SPBRG = 416 & 0xFF;
  SPBRGH = 416 >> 8;
  return;
}
SPBRG = 416 & 0xFF; -->Gives the same warning: (751) arithmetic overflow in constant expression
SPBRGH = 416 >> 8; --> has no complaints

Interesting because in the second case it looks like it evaluated the terms to the right of = and decided that they fit in a uint8_t but refused to do so in the first case.
 

Thread Starter

bug13

Joined Feb 13, 2012
2,002
It should not compile. In C there are lvalues and rvalues (for left and right). lvalules can be assigned to, rvalues cannot. A variable by itself is an lvalue. A variable casted is an rvalue, and hence cannot be assigned to. There is a way to cast on the left side of the expression. If you want to assign to the low byte of an int, for example:

*(unsigned char *)(&intvar) = x;

Will be accepted by the compiler (but will not likely help you out in getting rid of the error), This cast casts the address of the variable to be a pointer to a char, then defreferences that pointer to access a char.

Having worked on compilers professionally for over 30 years, including Microsoft C++ and Intel C++, I know a little bit about compilers. What is going on here is that the compiler is noticing the possibility of an overflow by looking only at the data types, before evaluating the expression, and hence the warning.
Thanks for sharing Bob, that's a bit of useful information for noob like me :).
 
Top