cancel
Showing results for 
Search instead for 
Did you mean: 

[BUG] STMCubeG4: Inefficient access in HAL_FDCAN_IRQHandler()

DSant.14
Associate

The below code cannot be reordered due to volatile storage type:

void HAL_FDCAN_IRQHandler(FDCAN_HandleTypeDef *hfdcan)
{
  uint32_t TxEventFifoITs;
  uint32_t RxFifo0ITs;
  uint32_t RxFifo1ITs;
  uint32_t Errors;
  uint32_t ErrorStatusITs;
  uint32_t TransmittedBuffers;
  uint32_t AbortedBuffers;
 
  TxEventFifoITs = hfdcan->Instance->IR & FDCAN_TX_EVENT_FIFO_MASK;
  TxEventFifoITs &= hfdcan->Instance->IE;
  RxFifo0ITs = hfdcan->Instance->IR & FDCAN_RX_FIFO0_MASK;
  RxFifo0ITs &= hfdcan->Instance->IE;
  RxFifo1ITs = hfdcan->Instance->IR & FDCAN_RX_FIFO1_MASK;
  RxFifo1ITs &= hfdcan->Instance->IE;
  Errors = hfdcan->Instance->IR & FDCAN_ERROR_MASK;
  Errors &= hfdcan->Instance->IE;
  ErrorStatusITs = hfdcan->Instance->IR & FDCAN_ERROR_STATUS_MASK;
  ErrorStatusITs &= hfdcan->Instance->IE;

This also leads to excess I/O access of registers. It should be replaced with something like this:

void HAL_FDCAN_IRQHandler(FDCAN_HandleTypeDef *hfdcan)
{
  uint32_t TxEventFifoITs;
  uint32_t RxFifo0ITs;
  uint32_t RxFifo1ITs;
  uint32_t Errors;
  uint32_t ErrorStatusITs;
  uint32_t TransmittedBuffers;
  uint32_t AbortedBuffers;
  uint32_t ir = hfdcan->Instance->IR;
  uint32_t ie = hfdcan->Instance->IE;
 
  TxEventFifoITs = ir & FDCAN_TX_EVENT_FIFO_MASK;
  TxEventFifoITs &= ie;
  RxFifo0ITs = ir & FDCAN_RX_FIFO0_MASK;
  RxFifo0ITs &= ie;
  RxFifo1ITs = ir & FDCAN_RX_FIFO1_MASK;
  RxFifo1ITs &= ie;
  Errors = ir & FDCAN_ERROR_MASK;
  Errors &= ie;
  ErrorStatusITs = ir & FDCAN_ERROR_STATUS_MASK;
  ErrorStatusITs &= ie;

The volatile I/O memory is read exactly once. All of the above memory assignments can then be reordered. Why is this important? We don't want to use up a ton of registers just for each of these AND operations, nor do we want to force the compiler to spill the registers to the stack. If they can be reodered, then the operations will be delayed until as late as possible and made more efficient as the CPU's individual instruction execution times can be considered when arranging the instructions.

0 REPLIES 0