Unacceptable GPIO behavior with optimization, Keil or Nordic issue?

[SRA]

#define PIN_INT1 26

void rx_event_read()
{
    volatile uint32_t pin_state = 0;

    //read the current pin state
    pin_state = nrf_gpio_pin_read(PIN_INT1);
    
    ...

Using Keil v5, ARMCC 5

Pin state equals:

Level 0 - 0x00000000 - pin low

Level3 breakpoint after read - 0x00000040

Level3 breakpoint before and stepped through - 0x50000000

This is almost shockingly unacceptable. Found this guy

nrf_gpio_pin_read and optimized code might return wrong value

Not Answered

Hi, I have found an interesting behavior from this code: nrf_gpio_cfg_input(7,NRF_GPIO_PIN_NOPULL); if (nrf_gpio_pin_read(7)==0) { NRF_LOG_INFO("ID1(P0.%d) is LOW.",7); }else{ NRF_LOG_INFO(…

By Tamisoft over 7 years ago in Nordic DevZone > Nordic Q&A

11 replies

Latest by Bruno Randolf over 5 years ago

but the issue seems very different. We're entering this function with the line held low, so are expecting a 0x00.

I'm not even sure how this is happening because the return value in read_pin is:

   return ((nrf_gpio_port_in_read(reg) >> pin_number) & 1UL);

I'm not sure if this is because Nordic is using a STATIC_INLINE. It pretty much doesn't seem like Nordic's fault because it's &1 so should be impossible to return a higher value. But this seems like something that would have to have been caught if Nordic is testing with optimization on/off.

Any idea what could be going on here!?

[Jørgen Holmefjord]

How are you reading the values of pin_state, and how have you configured the pin? I'm not able to reproduce this issue.

[SRA]

Jørgen, can you not see my code snippets? I just realized that the DevZone on iOS Safari does NOT show code, so thats... annoying.

#define PIN_INT1 26

void rx_event_read()
{
    volatile uint32_t pin_state = 0;

    //read the current pin state
    pin_state = nrf_gpio_pin_read(PIN_INT1);
    
    ...

[RK]

What are you doing with pin_state after the read? Are you actually making a decision based on it which you think is incorrect or are you just looking at it? This is optimized code, there's no guarantee the value exists as '0' after a read if the compiler can deduce it doesn't need it, or it doesn't need to shift it or mask it in order to make the decision later. 

You can't read optimized code as a direct transliteration of the underlying C and if you're going to step it you need to read the entire routine and deduce what's actually happening in the entirety. I'm pretty sure the chip and the compiler are working as designed, if you have a real issue, it's elsewhere .. 

And 'shockingly unacceptable' .. hyperbole. 

[SRA]

Yea... maybe you missed that it's a volatile uint32_t. And yes, it's making a decision. To top it off, there is a bitwise &1 that is clearly being ignored in the Nordic software. Their function is inline, maybe it should be volatile as well. I'm skeptical this underwent a lot of testing at higher optimization levels, and if that's the case, would be unacceptable.

[RK]

no I didn't miss it at all. The volatility of the int has nothing at all to do with whether the compiler chooses to actually perform the calculation and have that partial value available for inspection in a register or via a debugger. it  means it has to read the register and cannot optimise that out but it can reshuffle, move, change, reorder or modify the calculations performed provided they have the same effect later. So looking at what you think the value of the variable  is after in a debugger in release mode is not going to actually tell you much, not even if you have debug info in the binary (which will tell you roughly where the assembler you're looking  at comes in relation to your code .. roughly). 

If you think you have an actual issue with the decision made later, that's where you should be looking. I would myself check that the port does get read at some point and the value used in some way and if you really want to know what's going on, you need to read all the assembler and work it through.