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

Thread Starter

bug13

Joined Feb 13, 2012
1,853
Hi team

I got a warning [arithmetic overflow in constant expression] for the following code, what have I done wrong??

This is the line with warning:
Code:
            SPBRG =  (uint8_t) (416UL & 0x00FF);
Warnings:
uart.c:25:: warning: (751) arithmetic overflow in constant expression
Here is the full code:
Code:
void Uart_init(uart_params_t *params) {
  
    if (params == NULL) return;

    TRISCbits.TRISC7 = 1U; // per datasheet
    TRISCbits.TRISC6 = 1U; // per datasheet

    TXSTAbits.TXEN = 1U; // enable TX
    TXSTAbits.BRGH = 1U; // high speed

    RCSTAbits.CREN = 1U; // enable continuous RX

    BAUDCONbits.BRG16 = 1U; // enable 16-bit Baud rate

    /*
     * setup band rate
     * BR = FOSC/[4 * (n+1)], n = BRG
     */
    switch (params->br) {
        case BR_9600:
            SPBRGH = (uint8_t) (416UL >> 8U);
            SPBRG =  (uint8_t) (416UL & 0x00FF); // <<<<<<<<===== warning on this line
            break;
        case BR_14400:
            SPBRGH = (uint8_t) (227U >> 8U);
            SPBRG = (uint8_t) (227U & 0xFF);
            break;
        case BR_19200:
            SPBRGH = (uint8_t) (207U >> 8U);
            SPBRG = (uint8_t) (207U & 0xFF);
            break;
        case BR_38400:
            SPBRGH = (uint8_t) (103U >> 8U);
            SPBRG = (uint8_t) (103U & 0xFF);
            break;
        case BR_57600:
            SPBRGH = (uint8_t) (68U >> 8U);
            SPBRG = (uint8_t) (68U & 0xFF);
            break;
        default:
            SPBRGH = (uint8_t) (207U >> 8U);
            SPBRG = (uint8_t) (207U & 0xFF);
            break;
    }

    /* setup TX and RX interrupt */
    if (params->enableRXInt) PIE1bits.RCIE = 1U;
    if (params->enableTXInt) PIE1bits.TXIE = 1U;

    RCSTAbits.SPEN = 1U; // enable UART
}
 

nsaspook

Joined Aug 27, 2009
7,356
I thought I already musk off the number to within 0xFF with this (416UL & 0x00FF), this should not within the number of uint8_t??

So if this is the problem, how do I do it properly in XC8? I don't have the same complain in gcc somehow.
The 8-bit compiler should do the right thing here but xc8 is being "too smart"and not clairvoyant so it's warning you of a possible issue with integral promotional of operands that a times can't be fixed by a cast.

Try gcc with -pedantic
 

djsfantasi

Joined Apr 11, 2010
6,515
Does the value 416 need to be long?

I believe you need the two constants to be of the same types. I’d remove the unsigned long type assignment and recompile.
 

djsfantasi

Joined Apr 11, 2010
6,515
But 416UL and 255 (0X00FF) are two different sizes. I’d either remove the UL designation or cast both constants as uint16_t.
 

Thread Starter

bug13

Joined Feb 13, 2012
1,853
Does the value 416 need to be long?

I believe you need the two constants to be of the same types. I’d remove the unsigned long type assignment and recompile.
I still have warning with this:
SPBRG = (uint8_t) ((uint16_t)416U & (uint16_t)0x00FF);

But 416UL and 255 (0X00FF) are two different sizes. I’d either remove the UL designation or cast both constants as uint16_t.
Ooops, I actually thought unsigned long is 16-bit in XC8, after I looked it up in the manual, it's 32-bit. Fixed now, thanks!
 
This works with no warning
C:
#include <stdint.h>
void main(void) {
  uint8_t s;
  uint32_t s1;
  
  s1= (416UL & 0x00FF);
  s=(uint8_t)s1;
  
  return;
}

@djsfantasi it doesn't matter...this still throws the warning
C:
#include <stdint.h>
void main(void) {
  uint8_t s;
  
  s=(uint8_t) (416UL & 0x00FFUL);
  
  return;
}
I have to think about this more, but I think it is an xc8 peculiarity. If you look up this warning, it has drawn some legitimate complaints.
 

nsaspook

Joined Aug 27, 2009
7,356
This works with no warning
C:
#include <stdint.h>
void main(void) {
  uint8_t s;
  uint32_t s1;

  s1= (416UL & 0x00FF);
  s=(uint8_t)s1;

  return;
}

@djsfantasi it doesn't matter...this still throws the warning
C:
#include <stdint.h>
void main(void) {
  uint8_t s;

  s=(uint8_t) (416UL & 0x00FFUL);

  return;
}
I have to think about this more, but I think it is an xc8 peculiarity. If you look up this warning, it has drawn some legitimate complaints.
Normally a cast says to the compiler the programming knows what they are doing but xc8 knows 'better'.
 
Last edited:
@bug13

There are a couple of issues.

Compilers have warnings and they also typically have warning "levels". That means it can be very strict about warning you for everything it doesn't like, even though some may turn out to be what you intended, or it can only warn you about "big" things. I am being a little loose with the terminology, but basically, I like to use a default warning level or something more conservative unless I have a really good reason. So, I could probably tell you how to set the warning level on xc8 to make that warning go away. I don't advise doing that.

The second, and probably more important issue, is what the heck are you doing with this line...

SPBRG = (uint8_t) (416UL & 0x00FF);

You can calculate 416UL & 0x00FF and be done with it, right? So...
SPBRG = (uint8_t) (416UL & 0x00FF);
SPBRG = (uint8_t) (160);
SPBRG = (uint8_t) (0xA0);
//edit oops ignore the one I had here

Are all the same, except the first one, the one you used, gives you a warning.

Look at post #7 in this thread https://www.microchip.com/forums/m736903.aspx Now, that was 5 years ago, but I think something like that may be going on - but I don't know for sure.
 
Last edited:

nsaspook

Joined Aug 27, 2009
7,356
I agree. Even if the code works here the warning is saying on an 8-bit controller with 16-bit integers the better way is to have operand constants in the correct range for the expected 8-bit result register when integral promotions are used because the cast to 8-bits will not be correct with all types of overflows in intermediate operations.
 

Thread Starter

bug13

Joined Feb 13, 2012
1,853
@bug13

There are a couple of issues.

Compilers have warnings and they also typically have warning "levels". That means it can be very strict about warning you for everything it doesn't like, even though some may turn out to be what you intended, or it can only warn you about "big" things. I am being a little loose with the terminology, but basically, I like to use a default warning level or something more conservative unless I have a really good reason. So, I could probably tell you how to set the warning level on xc8 to make that warning go away. I don't advise doing that.

The second, and probably more important issue, is what the heck are you doing with this line...

SPBRG = (uint8_t) (416UL & 0x00FF);

You can calculate 416UL & 0x00FF and be done with it, right? So...
SPBRG = (uint8_t) (416UL & 0x00FF);
SPBRG = (uint8_t) (160);
SPBRG = (uint8_t) (0xA0);
//edit oops ignore the one I had here

Are all the same, except the first one, the one you used, gives you a warning.

Look at post #7 in this thread https://www.microchip.com/forums/m736903.aspx Now, that was 5 years ago, but I think something like that may be going on - but I don't know for sure.
Hi @Raymond Genovese,

I agree with you about not change the warning level, I usually like to deal with any warnings the compiler throw at me.

As why I did that code above was it was more readable for me. The value(416) was calculated with the formula in the datasheet, and eventually I want to write a function like this:

Code:
uint16_t uart_convert_baud_rate_to_register_value(uint16_t baud_rate);
I am building re-useable code here (slowly) and my above code will be something like this: (still don't know how to add IOs as function param easily tho)
Code:
void Uart_init(uart_params_t *params) {
    if (params == NULL) return;
    TRISCbits.TRISC7 = 1U; // per datasheet
    TRISCbits.TRISC6 = 1U; // per datasheet
    TXSTAbits.TXEN = 1U; // enable TX
    TXSTAbits.BRGH = 1U; // high speed
    RCSTAbits.CREN = 1U; // enable continuous RX
    BAUDCONbits.BRG16 = 1U; // enable 16-bit Baud rate
    /*
     * setup band rate
     * BR = FOSC/[4 * (n+1)], n = BRG
     */
     uint16_t br = uart_convert_baud_rate_to_register_value(params->br);
     SPBRGH = (uint8_t) (br  >> 8U);
     SPBRG =  (uint8_t) (br  & 0x00FF);

    /* setup TX and RX interrupt */
    if (params->enableRXInt) PIE1bits.RCIE = 1U;
    if (params->enableTXInt) PIE1bits.TXIE = 1U;
    RCSTAbits.SPEN = 1U; // enable UART
}
 

Thread Starter

bug13

Joined Feb 13, 2012
1,853
I agree. Even if the code works here the warning is saying on an 8-bit controller with 16-bit integers the better way is to have operand constants in the correct range for the expected 8-bit result register when integral promotions are used because the cast to 8-bits will not be correct with all types of overflows in intermediate operations.
I thought my code is within range?? Because I mask it off to stay within 0xFF?? As I mention I will ignore this warning, what I really meant was ignoring this warning in this case, but not ignoring ALL the [arithmetic overflow in constant expression] warnings in general

Or, I should still can't ignore this warning in this case even I mask it off to make sure it's within range? And I need to do something like this instead?
Code:
  uint8_t s;
  uint32_t s1;
  s1= (416UL & 0x00FF);
  s=(uint8_t)s1;
 
Last edited:

djsfantasi

Joined Apr 11, 2010
6,515
Ok, 416 decimal is 110100000 binary

416 & 0x00FF 10100000 binary
Or 160 decimal.

SBPRG = 160

This is the same conclusion that @Raymond Genovese came to. What is SPBRG declared as? You may need to cast 160 as the same type.
 
Top