Proposed code generation optimization

[BCoch]

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);

}

[Singh.Harjit]

​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]

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]

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]

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]

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]

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