Skip to main content
BCoch
BCoch
Senior
December 7, 2018
Question

Proposed code generation optimization

  • December 7, 2018
  • 4 replies
  • 939 views

It would be best if functions intended to be used to check status from within interrupt handlers are optimal. STM32CubeMX generates many utility functions which check whether a single bit is equal to that bit. It would probably be more efficient to simply check that the bit is non-zero.

For example:

__STATIC_INLINE uint32_t LL_I2C_IsActiveFlag_ADDR(I2C_TypeDef *I2Cx)

{

 return (READ_BIT(I2Cx->ISR, I2C_ISR_ADDR) == (I2C_ISR_ADDR));

}

would become:

__STATIC_INLINE uint32_t LL_I2C_IsActiveFlag_ADDR(I2C_TypeDef *I2Cx)

{

 return (READ_BIT(I2Cx->ISR, I2C_ISR_ADDR) != 0);

}

This topic has been closed for replies.

4 replies

Singh.Harjit
Singh.Harjit
Senior
December 9, 2018

​The generated code will be the same for either case.

The ST way allows for the definition of I2C_ISR_ADDR to change without any code changes. IMHO, it is the correct implementation.

BCoch
BCochAuthor
Senior
December 10, 2018

Hi Singh,

Thanks for considering my post. It seems to me that the definition of I2C_ISR_ADDR could change with no code changes in either case. I don't understand why you feel that isn't the case.

I tried compiling both options with no optimizations enabled and was surprised to see the implementation is indeed basically the same.

Original:

 8003a22: f003 0308 and.w r3, r3, #8

 8003a26: 2b08    cmp r3, #8

 8003a28: bf0c    ite eq

Proposed change:

 8003a48: f003 0308 and.w r3, r3, #8

 8003a4c: 2b00    cmp r3, #0

 8003a4e: bf14    ite ne

I would have expected that for the proposed change case, the and instruction would set the condition codes, rendering the compare instruction at 8003a4c unnecessary. Even so, if testing a bit not one of the least significant 8 bits, the proposed change method would still require only the two byte compare with zero instruction, versus longer instruction(s) for the current approach.

Best regards,

Bruce.

BCoch
BCochAuthor
Senior
December 10, 2018

Hi Singh,

When compiled in release mode, the generated assembly code is indeed 100% identical. I guess the compiler figures this out on it's own! Nice!

Best regards,

Bruce.

Singh.Harjit
Singh.Harjit
Senior
December 10, 2018

On your chip, I2C_ISR_ADDR is defined with a value of 0.

Let's say I make another chip that needs the same sequence by I2C_ISR_ADDR  has a value of 1.

With your suggested method, when this happens, they will have to re-read all the code and find every instance where it was compared to 0 and change it to 1. This is time consuming and error project.

With the ST method, they will change the definition of I2C_ISR_ADDR to 1 and when I compile code for the new chip, it will work fine.

BCoch
BCochAuthor
Senior
December 10, 2018

Hi Singh,

Both methods use the exact same method for reading the I2C_ISR_ADDR bit.

Namely: READ_BIT(I2Cx->ISR, I2C_ISR_ADDR)

Both methods continue to function fine, regardless of which bit position is being read.

Note that I2C_ISR_ADDR defines the bit mask. (And it is NOT defined as zero - it's 8, for bit 3.) READ_BIT returns the value of that bit, regardless of the position. And ultimately we only need to know whether or not that bit (the value returned by READ_BIT) was non-zero.

Best regards,

Bruce.

Singh.Harjit
Singh.Harjit
Senior
December 10, 2018

​It isn't about the value of I2C_ISR_ADDR but the concept that you don't write code with hardwired values.

Terms & ConditionsAccessibility statement