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

Discussion in 'Programmer's Corner' started by bug13, Apr 14, 2019.

  1. bug13

    Thread Starter Senior Member

    Feb 13, 2012
    1,610
    58
    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 (Text):
    1.             SPBRG =  (uint8_t) (416UL & 0x00FF);
    Warnings:
    Here is the full code:
    Code (Text):
    1. void Uart_init(uart_params_t *params) {
    2.  
    3.     if (params == NULL) return;
    4.  
    5.     TRISCbits.TRISC7 = 1U; // per datasheet
    6.     TRISCbits.TRISC6 = 1U; // per datasheet
    7.  
    8.     TXSTAbits.TXEN = 1U; // enable TX
    9.     TXSTAbits.BRGH = 1U; // high speed
    10.  
    11.     RCSTAbits.CREN = 1U; // enable continuous RX
    12.  
    13.     BAUDCONbits.BRG16 = 1U; // enable 16-bit Baud rate
    14.  
    15.     /*
    16.      * setup band rate
    17.      * BR = FOSC/[4 * (n+1)], n = BRG
    18.      */
    19.     switch (params->br) {
    20.         case BR_9600:
    21.             SPBRGH = (uint8_t) (416UL >> 8U);
    22.             SPBRG =  (uint8_t) (416UL & 0x00FF); // <<<<<<<<===== warning on this line
    23.             break;
    24.         case BR_14400:
    25.             SPBRGH = (uint8_t) (227U >> 8U);
    26.             SPBRG = (uint8_t) (227U & 0xFF);
    27.             break;
    28.         case BR_19200:
    29.             SPBRGH = (uint8_t) (207U >> 8U);
    30.             SPBRG = (uint8_t) (207U & 0xFF);
    31.             break;
    32.         case BR_38400:
    33.             SPBRGH = (uint8_t) (103U >> 8U);
    34.             SPBRG = (uint8_t) (103U & 0xFF);
    35.             break;
    36.         case BR_57600:
    37.             SPBRGH = (uint8_t) (68U >> 8U);
    38.             SPBRG = (uint8_t) (68U & 0xFF);
    39.             break;
    40.         default:
    41.             SPBRGH = (uint8_t) (207U >> 8U);
    42.             SPBRG = (uint8_t) (207U & 0xFF);
    43.             break;
    44.     }
    45.  
    46.     /* setup TX and RX interrupt */
    47.     if (params->enableRXInt) PIE1bits.RCIE = 1U;
    48.     if (params->enableTXInt) PIE1bits.TXIE = 1U;
    49.  
    50.     RCSTAbits.SPEN = 1U; // enable UART
    51. }
     
  2. nsaspook

    AAC Fanatic!

    Aug 27, 2009
    5,469
    6,060
  3. spinnaker

    AAC Fanatic!

    Oct 29, 2009
    7,780
    3,588
    And why are you anding two numeric values in code? That makes no sense at all and is inefficient.
     
  4. bug13

    Thread Starter Senior Member

    Feb 13, 2012
    1,610
    58
    Last edited: Apr 14, 2019
  5. bug13

    Thread Starter Senior Member

    Feb 13, 2012
    1,610
    58
    in BR_9600, I need to mask the number to be a byte (within 0xFF), the others are just copy and paste.
     
  6. nsaspook

    AAC Fanatic!

    Aug 27, 2009
    5,469
    6,060
    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
     
  7. djsfantasi

    AAC Fanatic!

    Apr 11, 2010
    4,344
    1,601
    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.
     
  8. djsfantasi

    AAC Fanatic!

    Apr 11, 2010
    4,344
    1,601
    But 416UL and 255 (0X00FF) are two different sizes. I’d either remove the UL designation or cast both constants as uint16_t.
     
  9. bug13

    Thread Starter Senior Member

    Feb 13, 2012
    1,610
    58
    I still have warning with this:
    SPBRG = (uint8_t) ((uint16_t)416U & (uint16_t)0x00FF);

    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!
     
  10. bug13

    Thread Starter Senior Member

    Feb 13, 2012
    1,610
    58
    I actually have no idea what this mean. You mean using a GCC compiler???
     
  11. Raymond Genovese

    Well-Known Member

    Mar 5, 2016
    1,513
    892
    This works with no warning
    Code (C):
    1.  
    2. #include <stdint.h>
    3. void main(void) {
    4.   uint8_t s;
    5.   uint32_t s1;
    6.  
    7.   s1= (416UL & 0x00FF);
    8.   s=(uint8_t)s1;
    9.  
    10.   return;
    11. }
    12.  

    @djsfantasi it doesn't matter...this still throws the warning
    Code (C):
    1.  
    2. #include <stdint.h>
    3. void main(void) {
    4.   uint8_t s;
    5.  
    6.   s=(uint8_t) (416UL & 0x00FFUL);
    7.  
    8.   return;
    9. }
    10.  
    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.
     
    bug13 likes this.
  12. nsaspook

    AAC Fanatic!

    Aug 27, 2009
    5,469
    6,060
    You said you didn't get overflow warnings in gcc, adding that line might cause them.
     
    bug13 likes this.
  13. nsaspook

    AAC Fanatic!

    Aug 27, 2009
    5,469
    6,060
    Normally a cast says to the compiler the programming knows what they are doing but xc8 knows 'better'.
     
    Last edited: Apr 15, 2019
    bug13 likes this.
  14. bug13

    Thread Starter Senior Member

    Feb 13, 2012
    1,610
    58
    Thanks guys! I think I will just ignore this warning and put some comments in there as well.
     
  15. Raymond Genovese

    Well-Known Member

    Mar 5, 2016
    1,513
    892
    @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: Apr 15, 2019
    nsaspook likes this.
  16. Raymond Genovese

    Well-Known Member

    Mar 5, 2016
    1,513
    892
    *sigh*
     
  17. nsaspook

    AAC Fanatic!

    Aug 27, 2009
    5,469
    6,060
    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.
     
  18. bug13

    Thread Starter Senior Member

    Feb 13, 2012
    1,610
    58
    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 (Text):
    1. 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 (Text):
    1. void Uart_init(uart_params_t *params) {
    2.     if (params == NULL) return;
    3.     TRISCbits.TRISC7 = 1U; // per datasheet
    4.     TRISCbits.TRISC6 = 1U; // per datasheet
    5.     TXSTAbits.TXEN = 1U; // enable TX
    6.     TXSTAbits.BRGH = 1U; // high speed
    7.     RCSTAbits.CREN = 1U; // enable continuous RX
    8.     BAUDCONbits.BRG16 = 1U; // enable 16-bit Baud rate
    9.     /*
    10.      * setup band rate
    11.      * BR = FOSC/[4 * (n+1)], n = BRG
    12.      */
    13.      uint16_t br = uart_convert_baud_rate_to_register_value(params->br);
    14.      SPBRGH = (uint8_t) (br  >> 8U);
    15.      SPBRG =  (uint8_t) (br  & 0x00FF);
    16.  
    17.     /* setup TX and RX interrupt */
    18.     if (params->enableRXInt) PIE1bits.RCIE = 1U;
    19.     if (params->enableTXInt) PIE1bits.TXIE = 1U;
    20.     RCSTAbits.SPEN = 1U; // enable UART
    21. }
     
  19. bug13

    Thread Starter Senior Member

    Feb 13, 2012
    1,610
    58
    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 (Text):
    1.   uint8_t s;
    2.   uint32_t s1;
    3.   s1= (416UL & 0x00FF);
    4.   s=(uint8_t)s1;
     
    Last edited: Apr 15, 2019
  20. djsfantasi

    AAC Fanatic!

    Apr 11, 2010
    4,344
    1,601
    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.
     
Loading...