Non-reentrant Functions as Macros for utilities used in both Main and Interrupts

Question

When compiling software with xc8 for a PIC which has functions which are called in both interrupts and in the main function the compiler gives an advisory:

non-reentrant function "foo" appears in multiple call graphs and has been duplicated by the compiler

Although this is not necessarily an issue, I understand from some reading that it is bad practice to call functions from both an interrupt and main.

Does this extend to pre-processor macros?

Most of the functions that I implement and are used in both are simple aliases for some simple maths, for example:

uint16_t timer3_get_value(void)
{
    return (uint16_t) (TMR3H << 8) | TMR3L;
}

Would making this function inline or changing it to a macro make a difference and/or be better practice?

#define TIMER3_GET_VALUE() (uint16_t) (TMR3H << 8) | TMR3L

score 2 · Accepted Answer · answered Sep 03 '19 at 17:28

2

Like the compiler message tells you, the code is duplicated. This has the very same effect as being inlined, put aside the overhead for call and return. Both versions are independent and can't influence each other.

So it's up to you to choose.

Relying on the compiler's capability makes your code less portable. And you get the compiler message which might be disturbing.
AFAIK inline just gives the compiler a hint. It does not have to obey it.
You best bet would be to use your last idea of making it a macro expression / statement.

For the latter this is common style:

#define function_name() do { /* anything it should do */ } while (0)

so you could use it like a void expression. If you accidentally try to assign it you'll get an error message. Please note the missing semicolon at the end. This will be provided at the place of the usage:

function_name();

answered Sep 03 '19 at 17:28

the busybee

2,140
8
20

Do you have a reference I can look at that says the the do-while is the common style for macro functions ? I have seen it before, but I did not know that it was a common way... – efox29 Sep 03 '19 at 18:50
Well, I found it in several well-known projects, a quick grep in the local MinGW installation shows headers of ncurses and python, for example. Why do you need a reference confirming that it's common style? It's a portable and straight forward way to ensure that the expansion can't be assigned. Before I found it I used to use parentheses as in `#define function_name() ( /* anything it should do */ )`. But that has problems... – the busybee Sep 04 '19 at 07:34
Maybe "reference" was the wrong word to use. I tend not to believe things on the internet unless I can find alternative evidence that support it. While what you say appears to be true, I am just looking for another confirmation. That's it. And like you, you're old method is how I currently use it - so before making the change, I'd like to see what the industry is going :) – efox29 Sep 04 '19 at 13:07

Dan1138 · Answer 2 · 2019-09-03T19:59:13.840

My observations here are specific to Microchip PIC 8-bit data path controllers and tend to go against common style in C programming.

These types of PIC controllers have fixed call stack depths from 2 to 32 nested calls, and no hardware to implement a data stack.

For these reasons C compilers like XC8 are quite complex when trying to produce efficient code from C source using modular programming style and encapsulation.

In general terms it is a bad idea to call a function from an interrupt handler on these types of PIC controllers.

The implementation choices of the Microchip compiler writers for XC8 have made creating re-entrant code hard to realize.

Suggest that:

Interrupt handlers not call functions at all.
Never allow interrupts to nest.
Keep the interrupt handler code execution time short.
Never ever use a delay loop in an interurpt handler.

A specific comment on your code:

uint16_t timer3_get_value(void)
{
    return (uint16_t) (TMR3H << 8) | TMR3L;
}

and:

#define TIMER3_GET_VALUE() (uint16_t) (TMR3H << 8) | TMR3L

These code fragments appear to logically do the same thing, but there is an issue when used for reading the count registers of PIC timers.

This problem comes from the PIC architecture using an 8-bit data bus. This prevents the movement of objects larger than 8-bits a single instruction cycle.

The most efficient assembly language generated from your code for reading TIMER3 is:

    MOVF    TMR3H,W
    MOVWF   RETURN_VALUE_H
    MOVF    TMR3L,W
    MOVWF   RETURN_VALUE_L

There are 2 instruction cycles between the reading of TMR3H and TMR3L. When TIMER3 is clocked at a rate equal to (or higher) than the instruction clock it is possible to read the wrong value for the high 8-bits of the TIMER3 count register.

Using the prescaler does not prevent this from happening but it will make it less likely to cause a problem.

Recient PIC controllers have timers implemented with a 16-bit read/write mode. Read the data sheet for your controller and look on the Microchip web site for example code.

Thanks for your input! The 16-bit reads still work the same at the register level, as the high-byte buffering is done in hardware, so the only change needed here is to read the lower byte first. — Harry Beadle, Sep 03 '19 at 23:20

Non-reentrant Functions as Macros for utilities used in both Main and Interrupts

2 Answers2