0

I am using C++ with the arm-none-eabi-gcc compiler and I am wondering why the new compiler version (10.3.1) decides to optimize out a function call which modifies a volatile variable while the old compiler version (7.3.1) does not.

Following are the relevant code parts extracted:

InterruptHandler.h:

class InterruptHandler : public iInterruptHandler {
 public:
  void registerInterruptCallback(iInterruptCallback *callback, int16_t interrupt);

 private:
  static volatile iInterruptCallback *callbackTable[callbackTableSize];

InterruptHandler.cpp:

volatile iInterruptCallback *InterruptHandler::callbackTable[callbackTableSize] = {nullptr};

void InterruptHandler::registerInterruptCallback(iInterruptCallback *callback, int16_t interrupt) {
  callbackTable[interrupt] = callback;
}

App.h:

class App {
 private:
   InterruptHandler interruptHandler;

App.cpp:

App::App() : interruptHandler() { 
  interruptHandler.registerInterruptCallback(&taskingController, TASK_HANDLER_IRQ);
}

In total the function "registerInterruptCallback" is called 4 times, of which one function call is optimmized out. The pointers in "callbackTable" are then used in the InterruptCallback to call the relevant function for the interrupt, which is then, of course, not working if there is no pointer available due to optimization.

My assumption was that the compiler is not allowed to optimize the function call because the variable is declared as volatile.

Is it correct behavior of the compiler and if yes, why?

Additional information: I tried to give the function "registerInterruptCallback" the attribute for no optimization __attribute__((optimize("O0"))) which is then working, but I thought this should be solvable with standard C++ means.

Martin
  • 11
  • 1
  • 4
    `callbackTable[interrupt]` is *not* volatile. If you need it to be volatile, you should declare `static iInterruptCallback * volatile callbackTable[callbackTableSize];` – n. m. could be an AI Jun 21 '23 at 09:50
  • i think you should declare registerInterruptCallback as static too. static void registerInterruptCallback(iInterruptCallback *callback, int16_t interrupt); – Ahmed Anter Jun 21 '23 at 10:10
  • 4
    Also `volatile` keyword has meaning: value of variable can change outside of CPU control. Having this keyword in context of something which name contains `callback` is a code smell. I suspect you have multi threading environment. In such case use of `volatile` is bad thing. Please specify why you use this keyword. – Marek R Jun 21 '23 at 10:40
  • @n.m.willseey'allonReddit Thank you very much for this fast and helpful answer! This solved the issue. I was not really aware of this, but actually makes sense when I think about it. – Martin Jun 21 '23 at 11:50
  • @MarekR The code runs on a single core MCU, so I think it is not multi threading. The intention is to once register all modules callback functions at startup in that table and use the table during runtime within the ISR to call the related callback function. Do you see a problem with this? – Martin Jun 21 '23 at 11:57
  • I do not have experience with MCU devices, but if I understand `volatile` is useful code reads some well defined fragment of memory where other device input/output is hidden. So `volatile` variable is used to communicate with some device. Here your variable is not such thing. Most probably later you are registering this table using some API so when interrupt is triggered proper function is called. It would be best if you check API documentation if `volatile` is recommended in this case. – Marek R Jun 21 '23 at 12:42

0 Answers0