Optimized code stops working

[Jose Xavier]

On the following code I have a packet of 6 bytes that I write to a buffer. I pass the buffer as read_packet function argument. When optimized, the code doesn't work. I only got noise on the UART and even with the debugger I coudn't see the correct values. After some hours of trial and error I decided to select the level 0 of optimization and it started working.

What is the compiler doing?

while (true)
{
    uint32_t received[6];
		
			//received
			read_packet(received);
			//nrf_gpio_pin_toggle(led1);
			//nrf_gpio_pin_toggle(led3);
			app_uart_put(received[1]);
			//app_uart_put(12);
    
    //printf("The contents of the package is %u\n\r", (unsigned int)received);
}

bool read_packet(uint32_t *buffer)
{
    //uint32_t *result = malloc(sizeof(uint32_t)*6);
		static uint32_t result[6];

    NRF_RADIO->EVENTS_READY = 0U;
    // Enable radio and wait for ready
    NRF_RADIO->TASKS_RXEN = 1U;

    while (NRF_RADIO->EVENTS_READY == 0U)
    {
        // wait
    }
    NRF_RADIO->EVENTS_END = 0U;
    // Start listening and wait for address received event
    NRF_RADIO->TASKS_START = 1U;

    // Wait for end of packet or buttons state changed
    while (NRF_RADIO->EVENTS_END == 0U)
    {
        // wait
    }

    if (NRF_RADIO->CRCSTATUS == 1U)
    {
      buffer[0] = packet[0];
			buffer[1] = packet[1];
			buffer[2] = packet[2];
			buffer[3] = packet[3];
			buffer[4] = packet[4];
			buffer[5] = packet[5];
    }
    NRF_RADIO->EVENTS_DISABLED = 0U;
    // Disable radio
    NRF_RADIO->TASKS_DISABLE = 1U;

    while (NRF_RADIO->EVENTS_DISABLED == 0U)
    {
        // wait
    }
    return 1;
}

[Susheel Nuguru]

uint32_t received[6] 

change this to

uint8_t received[6]; 

and

bool read_packet(uint32_t *buffer)

change to

bool read_packet(uint8_t *buffer)

[RK]

No there's nothing wrong with that code. It's odd, admittedly, taking the uint8_t and putting them into a uint32_t array, but it's quite legitimate code. I assumed that was why he has the 6 individual array assignments instead of a memcpy().

the buffer pointer 'packet' itself is a uint8_t and that's the one handed off to the RADIO module so it's fine. That's the one which needs to be a byte pointer.

[Susheel Nuguru]

given the code he showed here, there is nothing else optimizable apart from those. So if it works with no optimizations it has to be either variables or arguments. Rest of all the code are volatile device registers. Agree that it is legitimate to copy uint8_t into uint32_t. What happens if you put uint32_t in uint8_t in app_uart_put(received[1]), i am guessing this should be ok too as only LSB is copied and the rest ignored?

[RK]

I can't see anything with that code which would be affected under optimisation, at all. Which makes me wonder if that's really where the problem is or if it's elsewhere entirely and what we're seeing is the aftermath of a debugging session's worth of edits.

I agree all the registers are volatile, all the loops should remain, all the values should be read and not optimized out. Without seeing the assembler it's hard to know what's going on.

The only 'clue' was in the second message from the OP who said packet has data but buffer doesn't, so I thought CRCSTATUS

sending received[1] to app_uart_put is fine, a byte is expected so the compiler should mask off the top and send it in r0 or assume the routine will mask it somewhere itself. It's a one-argument function so it would get passed in the full 32bit r0 either way

I agree odd to use uint32_t[] but not incorrect that I can tell.

[Susheel Nuguru]

Agree, it does not look like the optimization problem is from this code.