STMicroelectronics / STM32CubeL4

STM32Cube MCU Full Package for the STM32L4 series - (HAL + LL Drivers, CMSIS Core, CMSIS Device, MW libraries plus a set of Projects running on all boards provided by ST (Nucleo, Evaluation and Discovery Kits))
Other
262 stars 153 forks source link

const array inside functions should be static #16

Closed CanastraRF closed 2 years ago

CanastraRF commented 3 years ago

const array inside functions should be static. Otherwise the compiler generate an array in the flash and copy this array to the stack. This needs additional stackspace and time to copy this const to the stack.

Example: stm32l4xx_hal_dcmi.c: static uint32_t DCMI_TransferSize(uint32_t InputSize) { uint32_t j = 1; uint32_t temp = InputSize; uint32_t aPrime[NPRIME] = {0}; uint32_t output = 2; / Want a result which is an even number / static const uint32_t PrimeArray[NPRIME] = { 1UL, 2UL, 3UL, 5UL,7UL, 11UL, 13UL, 17UL,19UL, 23UL, 29UL, 31UL,37UL, 41UL, 43UL, 47UL};

stm32l4xx_hal_irda.c: static HAL_StatusTypeDef IRDA_SetConfig(IRDA_HandleTypeDef *hirda) { uint32_t tmpreg; IRDA_ClockSourceTypeDef clocksource; HAL_StatusTypeDef ret = HAL_OK;

if defined(USART_PRESC_PRESCALER)

static const uint16_t IRDAPrescTable[12] = {1U, 2U, 4U, 6U, 8U, 10U, 12U, 16U, 32U, 64U, 128U, 256U};

endif / USART_PRESC_PRESCALER /

stm32l4xx_hal_smartcard.c: static HAL_StatusTypeDef SMARTCARD_SetConfig(SMARTCARD_HandleTypeDef *hsmartcard) { uint32_t tmpreg; SMARTCARD_ClockSourceTypeDef clocksource; HAL_StatusTypeDef ret = HAL_OK;

if defined(USART_PRESC_PRESCALER)

static const uint16_t SMARTCARDPrescTable[12] = {1U, 2U, 4U, 6U, 8U, 10U, 12U, 16U, 32U, 64U, 128U, 256U};

endif / USART_PRESC_PRESCALER /

stm32l4xx_hal_smartcard_ex.c: static void SMARTCARDEx_SetNbDataToProcess(SMARTCARD_HandleTypeDef hsmartcard) { uint8_t rx_fifo_depth; uint8_t tx_fifo_depth; uint8_t rx_fifo_threshold; uint8_t tx_fifo_threshold; / 2 0U/1U added for MISRAC2012-Rule-18.1_b and MISRAC2012-Rule-18.1_d */ static const uint8_t numerator[] = {1U, 1U, 1U, 3U, 7U, 1U, 0U, 0U}; static const uint8_t denominator[] = {8U, 4U, 2U, 4U, 8U, 1U, 1U, 1U};

stm32l4xx_hal_uart_ex.c: static void UARTEx_SetNbDataToProcess(UART_HandleTypeDef *huart) { uint8_t rx_fifo_depth; uint8_t tx_fifo_depth; uint8_t rx_fifo_threshold; uint8_t tx_fifo_threshold; static const uint8_t numerator[] = {1U, 1U, 1U, 3U, 7U, 1U, 0U, 0U}; static const uint8_t denominator[] = {8U, 4U, 2U, 4U, 8U, 1U, 1U, 1U};

stm32l4xx_hal_usart_ex.c: static void USARTEx_SetNbDataToProcess(USART_HandleTypeDef husart) { uint8_t rx_fifo_depth; uint8_t tx_fifo_depth; uint8_t rx_fifo_threshold; uint8_t tx_fifo_threshold; / 2 0U/1U added for MISRAC2012-Rule-18.1_b and MISRAC2012-Rule-18.1_d */ static const uint8_t numerator[] = {1U, 1U, 1U, 3U, 7U, 1U, 0U, 0U}; static const uint8_t denominator[] = {8U, 4U, 2U, 4U, 8U, 1U, 1U, 1U};

Please fix this

Reto Felix

RKOUSTM commented 3 years ago

Hi @CanastraRF,

Thank you for this report. Your request has been forwarded to our development teams. We will get back to you as soon as they provide us with their feedback.

With regards,

RKOUSTM commented 3 years ago

Hi @CanastraRF,

First of all we would like to thank you for your contribution.

Actually, the point you raised is already known. It has been internally fixed. The fix will be published in the frame of a future publication. We cannot share a date for the moment.

With regards,

RKOUSTM commented 3 years ago

ST Internal Reference: 93869

RKOUSTM commented 3 years ago

Hi @CanastraRF,

First, allow me to thank you for your contribution. However, the final decision for our technical committee is no need to add the static qualifier for the const tables inside functions. Indeed, this decision is made on the basis of the following rule of usage:

Basically, the only difference between the two is how many times the variable is initialized. Statics are initialized only once. For plain old data types where the right-hand side is constant, we won't see any difference. i.e.,

void foo() {
    static const double PI = 3.14;
    const double PI = 3.14;
}

But if the right-hand side is non-trivial, we could see a performance difference:

void foo() {
    static const unsigned long long fib100 = fibonacci( 100 );
    const unsigned long long fib100 = fibonacci( 100 );

In the first case, fib100 is computed exactly once during the lifetime of the program. In the second case, it is computed each time the foo() is called. If fibonacci() is poorly coded, this could result in a significant performance degradation.

So, It is not useful to put static const instead of const and all CRs for this point will be closed with the status INVALID.

Please, allow me to close this issue. Thank you for your comprehension.

With regards,

CanastraRF commented 3 years ago

Dear Support Thanks for your explanation.

If the constant will be the same every time the function is called, use static const.

All my sample are from this case. So please add static in this case.

So I hope this is fix in next release.

You can close this case.

With regards,

RKOUSTM commented 3 years ago

ST Internal Reference: 98000

RKOUSTM commented 3 years ago

Hi @CanastraRF,

The fix you requested has been implemented and is available in the frame of the latest STM32CubeL4 FW package V1.17.0 release. Thank you again for your contribution.

With regards,

CanastraRF commented 3 years ago

Hi @RKOUSTM I download the current version from https://www.st.com/en/embedded-software/stm32cubel4.html and I check the current version on https://github.com/STMicroelectronics/STM32CubeL4/tree/master/Drivers/STM32L4xx_HAL_Driver/Src In both case the following is not fixed.

stm32l4xx_hal_dcmi.c:
static uint32_t DCMI_TransferSize(uint32_t InputSize)
{
  uint32_t j = 1;
  uint32_t temp = InputSize;
  uint32_t aPrime[NPRIME] = {0};
  uint32_t output = 2; /* Want a result which is an even number */
  **static const** 
  uint32_t PrimeArray[NPRIME] = { 1UL,  2UL,  3UL,  5UL,
                                7UL, 11UL, 13UL, 17UL,
                               19UL, 23UL, 29UL, 31UL,
                               37UL, 41UL, 43UL, 47UL};

Please fix this too

RKOUSTM commented 3 years ago

Hi @CanastraRF,

This issue has already been raised internally only for UART, USART, IRDA, SMARTCARD drivers. The reported point has been transferred to our development teams to take into consideration. The fix will be made available in the frame of a future release.

Thank you for your contribution. We are looking forward to reading from you again.

With regards,

RKOUSTM commented 3 years ago

ST Internal Reference: 111969

ALABSTM commented 2 years ago

Hi @CanastraRF,

I hope you are fine. The issue you reported has been fixed in the frame of version v1.17.1 of the STM32CubeL4 published recently on GitHub, as you can see below. Thank you again for having reported.

https://github.com/STMicroelectronics/STM32CubeL4/blob/f93a2f74f8e9912405dbf1a297b6df0c423eddf2/Drivers/STM32L4xx_HAL_Driver/Src/stm32l4xx_hal_dcmi.c#L1468

With regards,

CanastraRF commented 2 years ago

Thank you for the fix