Inefficient code?

Thread Starter

ApacheKid

Joined Jan 12, 2015
1,762
Look at this code, this is existing code used when building an app for the STM device F446RE, a simple LED blinker.

1666027375987.png

Not sure if this is STM authored code or what, but look at it.

In my case GPIO_NUMBER happens to be 16, so it loops 16 times before it finds iocurrent == ioposition, that seems rather costly to me.

Why not simply compute ioposition rather than looping like that?
 

Attachments

MrChips

Joined Oct 2, 2009
34,814
Most modern software IDE platforms do this.

HAL stands for Hardware Abstraction Layer. The purpose of HAL and many CMSIS (Common Microcontroller Software Interface Standard) libraries is intended to provide the programmer with generalized functions that can be used with specific I/O devices without having to constantly modify code when I/O assignments change.

Yes, library functions can be very inefficient. This goes back to the debate between ASM and HLL (high level language).
For maximum code size and speed efficiency, write in ASM.
For improved programmer efficiency, write in HLL.
You can bypass library functions by writing directly to HW registers, for examples,

GPIOB->ODR = 0x0A05; // set GPIOB to 0000 1010 0000 0101

GPIOC->BSRRL = GPIO_Pin_15; // set PC15 to 1
 

Thread Starter

ApacheKid

Joined Jan 12, 2015
1,762
Most modern software IDE platforms do this.

HAL stands for Hardware Abstraction Layer. The purpose of HAL and many CMSIS (Common Microcontroller Software Interface Standard) libraries is intended to provide the programmer with generalized functions that can be used with specific I/O devices without having to constantly modify code when I/O assignments change.

Yes, library functions can be very inefficient. This goes back to the debate between ASM and HLL (high level language).
For maximum code size and speed efficiency, write in ASM.
For improved programmer efficiency, write in HLL.
You can bypass library functions by writing directly to HW registers, for examples,

GPIOB->ODR = 0x0A05; // set GPIOB to 0000 1010 0000 0101

GPIOC->BSRRL = GPIO_Pin_15; // set PC15 to 1
yes that's all true, I was referring to the loop itself, it loops for one reason only, to reach a state where iocurrent == ioposition, but that same state could probably be just computed, like its trying to find the 5 in 32 (i.e. given 32 its trying to find 5) but does so by repetitive bit shifting.
 

MrChips

Joined Oct 2, 2009
34,814
I looked more closely at what the HAL_GPIO_Init( ) function does.

This function initializes the GPIO_Init structure based on the information you have given it.
For example, you could be configuring GPIO_Pin14, GPIO_Pin5, GPIO_Pin4, GPIO_Pin_0 all at the same time. It is trying to find which pins you want initialized. It can be any random selection of pins.

My conclusion, the function does what it is supposed to do. It is not inefficient and I cannot think of any other way of coding this that is more efficient.
 

Thread Starter

ApacheKid

Joined Jan 12, 2015
1,762
I looked more closely at what the HAL_GPIO_Init( ) function does.

This function initializes the GPIO_Init structure based on the information you have given it.
For example, you could be configuring GPIO_Pin14, GPIO_Pin5, GPIO_Pin4, GPIO_Pin_0 all at the same time. It is trying to find which pins you want initialized. It can be any random selection of pins.

My conclusion, the function does what it is supposed to do. It is not inefficient and I cannot think of any other way of coding this that is more efficient.
OK I see, you might well be right.

I did not realize that GPIO_Init.Pin could be used to specify multiple pins at once, names are often everything, naming that "Pins" would have saved me quite some time pondering this.

Thanks!
 
Last edited:
Top