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

WBahn

Joined Mar 31, 2012
30,045
Great suggestion, @WBahn ! I see where you are going.

I must admit that I’m confounded by the effort in eliminating these warnings. As my grandfather might say, they’re constants for cripes sake! There’s no reason not to perform the calculation manually and define the variable in the code as a const with a value of 160 (decimal). If you want to remember how that value was determined, the correct place is not in the code but with a comment.
Actually, you can make a good argument for doing the calculation in the code precisely because it lets the code do the calculations and not the error-prone human that is trying to modify/maintain the code.

Keep in mind that most compilers will evaluate the constant at compile time, so there is no runtime penalty in having an expression calculate the final value.

I would make the constants that might change constant-like macros or make it a function-like macro with the constant value as the argument.

Beyond that, however, trying to understand exactly why this compiler is throwing this warning is of value, particularly if you are going to be using this compiler in the future, so that you can better understand what is actually causing warnings to be thrown so that you can better determine whether it is really telling you something you need to pay attention to in a particular instance and, if it isn't, how you can best make the warning go away in the least kludgy way. I ascribe to the opinion that code should compiler warning free (using the most restrictive warning level that is tolerable). Only when absolutely unavoidable should warnings be accepted and documented.
 

djsfantasi

Joined Apr 11, 2010
9,160
I should have asked long ago. I quickly went through the posts, but can’t find what I’m looking for.

How is SBPRG declared?​
 

Thread Starter

bug13

Joined Feb 13, 2012
2,002
I should have asked long ago. I quickly went through the posts, but can’t find what I’m looking for.

How is SBPRG declared?​
MCU PIC18F24J10

Code:
// Register: SPBRG
#define SPBRG SPBRG
extern volatile unsigned char           SPBRG               __at(0xFAF);
From datasheet:

Capture.PNG
 

djsfantasi

Joined Apr 11, 2010
9,160
Some additional information.. I understand that your compiler is throwing a warning. But I decided to duplicate the code, as I believe it should be written, in my compiler.

C:
{
  uint8_t SBPRG;
  SBPRG = (uint8_t)((uint16_t)(416) & (uint16_t)(0xFF));
}
This compiles with no errors and no warnings. Can you copy it and see if it causes a warning in your compiler? In all of your posted code, I haven't seen this combination.
 

Thread Starter

bug13

Joined Feb 13, 2012
2,002
Some additional information.. I understand that your compiler is throwing a warning. But I decided to duplicate the code, as I believe it should be written, in my compiler.

C:
{
  uint8_t SBPRG;
  SBPRG = (uint8_t)((uint16_t)(416) & (uint16_t)(0xFF));
}
This compiles with no errors and no warnings. Can you copy it and see if it causes a warning in your compiler? In all of your posted code, I haven't seen this combination.
Still have warning, image attached.

Capture.PNG
 

Attachments

spinnaker

Joined Oct 29, 2009
7,830
Still have warning, image attached.
Once again. Why are you computing the value????

Either set the value as I recommend or see if the code works your way and don't worry about the warning. Some of these compilers suck at trying to typecast to remove the warnings. I gave up long ago trying to eliminate all warnings. You can get warnings inside microchip library code.


BTW Did you mention what compiler and version you are using?

This works in XC8 V1.45
SPBRG = ((unsigned char)416) & 0x00FF;
 

djsfantasi

Joined Apr 11, 2010
9,160
Don't think it's a good idea, SPBRG is hardware register.
Until you confirm that it’s working as you want, I have one more comment. I don’t see why it’s not a good idea. You're not changing the definition of the register. All you are doing is telling the compiler what you expect the output to look like. I may be wrong, but others may correct me.
 

spinnaker

Joined Oct 29, 2009
7,830
You could also try this

unsigned a=416;
SPBRG = (unsigned char)(a & 0x00FF);
If that doesn't work then there is a compiler bug.
 

Thread Starter

bug13

Joined Feb 13, 2012
2,002
Until you confirm that it’s working as you want, I have one more comment. I don’t see why it’s not a good idea. You're not changing the definition of the register. All you are doing is telling the compiler what you expect the output to look like. I may be wrong, but others may correct me.
I tried this:
Code:
(uint8_t)(SPBRG) = (uint8_t) (416 & 0x00FF);
It didn't compile:
Code:
uart.c:27:13: error: assignment to cast is illegal, lvalue casts are not supported
            (uint8_t)(SPBRG) = (uint8_t) (416 & 0x00FF);
            ^~~~~~~~~~~~~~~~ ~
1 error generated.
 

Thread Starter

bug13

Joined Feb 13, 2012
2,002
Did you try this?

SPBRG = (416u) & 0x00FF;
This does not give me a warning.
Still same warning.

You could also try this

unsigned a=416;
SPBRG = (unsigned char)(a & 0x00FF);
If that doesn't work then there is a compiler bug.
This is a bit strange, it still give me the same warning, but at a wrong location (at the end of the curly brackets in the function, instead of at the exact line of code above)

I think we found a bug!
 

spinnaker

Joined Oct 29, 2009
7,830
Still same warning.



This is a bit strange, it still give me the same warning, but at a wrong location (at the end of the curly brackets in the function, instead of at the exact line of code above)

I think we found a bug!

The second has to work. How on earth then could you get the low byte of anything? Must be a compiler bug, Bug. ;)

Why not just drop down a version or two in compiler?

Did you try posting on the microchip forum?


I would post just the code that causes you the error. Use both examples.
 

spinnaker

Joined Oct 29, 2009
7,830
Still same warning.



This is a bit strange, it still give me the same warning, but at a wrong location (at the end of the curly brackets in the function, instead of at the exact line of code above)

I think we found a bug!
Try also simplifying your code. Create a new project with nothing but the xc,h include, configuration bits and the code line causing you the issue. Even if it still gives a waring, it will be a nice simple example to post on microchip.
 

djsfantasi

Joined Apr 11, 2010
9,160
I tried this:
Code:
(uint8_t)(SPBRG) = (uint8_t) (416 & 0x00FF);
It didn't compile:
Code:
uart.c:27:13: error: assignment to cast is illegal, lvalue casts are not supported
            (uint8_t)(SPBRG) = (uint8_t) (416 & 0x00FF);
            ^~~~~~~~~~~~~~~~ ~
1 error generated.
This is instructive. The datasheet you posted indicated the SBPRG is an 8 bit variable. So why is the compiler giving an error that lvalue casts are not supported?
 

nsaspook

Joined Aug 27, 2009
13,260
Still same warning.



This is a bit strange, it still give me the same warning, but at a wrong location (at the end of the curly brackets in the function, instead of at the exact line of code above)

I think we found a bug!
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.
 

Thread Starter

bug13

Joined Feb 13, 2012
2,002
Here is my full code if anyone want to keep investigating, it's pretty clean (so as I believe):

Environment:
  • MPLAB X v5.15
  • XC8 2.05, C99, free mode
  • Win8
  • Anything else, just ask.

Code:
// PIC18F24J10 Configuration Bit Settings

// 'C' source line config statements

// CONFIG1L
#pragma config WDTEN = OFF      // Watchdog Timer Enable bit (WDT disabled (control is placed on SWDTEN bit))
#pragma config STVREN = ON      // Stack Overflow/Underflow Reset Enable bit (Reset on stack overflow/underflow enabled)
#pragma config XINST = OFF      // Extended Instruction Set Enable bit (Instruction set extension and Indexed Addressing mode disabled (Legacy mode))

// CONFIG1H
#pragma config CP0 = ON         // Code Protection bit (Program memory is code-protected)

// CONFIG2L
#pragma config FOSC = HS        // Oscillator Selection bits (HS oscillator)
#pragma config FOSC2 = ON       // Default/Reset System Clock Select bit (Clock selected by FOSC as system clock is enabled when OSCCON<1:0> = 00)
#pragma config FCMEN = ON       // Fail-Safe Clock Monitor Enable bit (Fail-Safe Clock Monitor enabled)
#pragma config IESO = ON        // Two-Speed Start-up (Internal/External Oscillator Switchover) Control bit (Two-Speed Start-up enabled)

// CONFIG2H
#pragma config WDTPS = 32768    // Watchdog Timer Postscale Select bits (1:32768)

// CONFIG3H
#pragma config CCP2MX = DEFAULT // CCP2 MUX bit (CCP2 is multiplexed with RC1)

// #pragma config statements should precede project file includes.
// Use project enums instead of #define for ON and OFF.
Code:
/* uart.h */

/*
* File:   uart.h
* Author: james
*
* Created on 10 April 2019, 10:35 AM
*/

#ifndef UART_H
#define    UART_H

#ifdef    __cplusplus
extern "C" {
#endif

#include <stdint.h>

    typedef enum {
        BR_9600 = 1, // error = -0.08%
        BR_14400, // error = -0.08%
        BR_19200, // error = 0.16%
        BR_38400, // error = 0.16%
        BR_57600 // error = 0.644%   
    } uart_br_t;

    typedef struct {
        uart_br_t br;
        uint8_t enableRXInt : 1;
        uint8_t enableTXInt : 1;
    } uart_params_t;

    void Uart_init(uart_params_t *);
    uint8_t Uart_isTxReady(void);
    uint8_t Uart_get(void);
    void Uart_putc(uint8_t);

#ifdef    __cplusplus
}
#endif

#endif    /* UART_H */
Code:
/* uart.c */

#include <stddef.h>
#include <xc.h>
#include "uart.h"

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: // n = 416
            SPBRGH = 0x01;
            SPBRG = 0xA0;
//            SPBRGH = (uint8_t) (416U >> 8U);
//            SPBRG =  (uint8_t)(416U & 0x00FF);
            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
}

uint8_t Uart_isTxReady(void) {
    return TXSTAbits.TRMT;
}

uint8_t Uart_get(void) {
    return RCREG;
}

void Uart_putc(uint8_t data) {
    TXREG = data;
}
 
Top