Yet another optimization problem

Go To Last Post
8 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
typedef struct {
	volatile uint8_t* port;
	uint8_t pin;
} pin;
typedef struct {
	uint8_t movement;
	uint8_t afterlife;
	bool smooth;
	uint16_t slowFactor;
	uint16_t strength;
	uint32_t limit;
	bool useLimit;
	volatile uint32_t progress;
	bool nonExistant;
	struct {
		pin choice;
		pin o1;
		pin o2;
		pin o3;
		pin o4;
	} ports;
} tStepper;
int i;
volatile tStepper stepper [ stepperCount ];

But if in an interrupt I do:

for ( i = 0; i < stepperCount; i++ ) stepper[i].progress++;

The progress++ is removed by the optimizer although both the entire array AND the progress variable are volatile.

Is that a bug in gcc or I am doing it the wrong way?

-Angel Angelov
"Humans are stupid."
I'm probably working on a scratch-build rocket or on a PCB

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Are you sure?

The 'progress' field should get incremented whether it is volatile or not.

Can you post a minimal compilable program?

David.

Edit. I have experimented with your code, and come to some conclusions.
1. The ISR() will increment the 'progress' fields regardless of any volatile quantifier.
2. The foreground code in main() will ONLY re-read the value of the progress field IF 'global' is volatile AND some part of the 'stepper' is declared volatile.
3. I would have thought that an assignment of a volatile expression should not be optimised even if 'global' was not volatile.

So I would suggest that you make sure any LHS of a volatile assignment is declared volatile.

#include 
#include 
#include 
#include 

typedef struct {
    volatile uint8_t *port;
    uint8_t pin;
} pin;
typedef struct {
    uint8_t movement;
    uint8_t afterlife;
    bool smooth;
    uint16_t slowFactor;
    uint16_t strength;
    uint32_t limit;
    bool useLimit;
    volatile uint32_t progress;
    bool nonExistant;
    struct {
        pin choice;
        pin o1;
        pin o2;
        pin o3;
        pin o4;
    } ports;
} tStepper;
#define stepperCount 4
int i;
volatile int global;
tStepper stepper[stepperCount];

ISR(TIMER1_OVF_vect)
{
    PORTB = 0;
    for (i = 0; i < stepperCount; i++)
        stepper[i].progress++;
}

void main(void)
{
    TCCR1A = 0x01;        // just make an OVF IRQ
    TCCR1B = 0x0C;
    TIMSK |= (1 << TOIE1);
    sei();
    while (1)
        global = stepper[0].progress;
}

I am unsure of the exact semantics of the volatile assignment. But declaring both sides volatile seems to be a workaround.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

David,

Supposing your:

#define stepperCount 4

was actually:

static int stepperCount;

I have a feeling the optimiser would see it as being a constant 0 as far as ISR(TIMER1_OVF_vect) was concerned and would not generate the for() code.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You cannot declare a dynamic array.

tStepper stepper[stepperCount];

So I guessed that this must be a #define.

I suppose if it was a single member array, a clever optimiser would see that it was a single iteration loop.

As far as I can see, ISR()'s always get compiled ok. The problem really arises in the foreground code. A really clever optimiser can deduce a volatile expression. An instance of an AVR sfr in an expression is always recognised as a volatile expression.

You normally know when you are assigning a volatile expression, and can specify the receiving variable as volatile.

If I had the enthusiasm, I could try assigning a volatile expression to a non-volatile variable on a different platform. I am sure this should all be in the ANSI specs. Again, I have not the energy.

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

global = stepper[0].progress;

voltaile isn't going to protect you from the above problem. Apart from on side being uint32 and the other being int, it's a multibyte variable so you'll have atomicity problems. Been discussed many times before - seems to be my pet problem..........

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks for helping.
So, I need to assign progress to some other volatile variable in the main program body so that it's not optized out?

Sorry for my bad english.

-Angel Angelov
"Humans are stupid."
I'm probably working on a scratch-build rocket or on a PCB

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

No. The ISR() happily increments your 'progress' field. And an ISR() always operates with IRQs disabled, so you have no trouble with atomicity there. I cannot reproduce your 'stepper[i].progress++;' ever being removed.

Your foreground code is where the optimiser may interfere. If you declare both sides of the assignment volatile, then it will not be removed.

To be safe from atomicity issues, just encase any foreground references within a cli/sei pair or use an atomicity macro.

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Ooooooooooooooh I'm soo embarassed!
I had put = instead of == and that caused the for cycle to always exit and so it was removed by the optimizer...

-Angel Angelov
"Humans are stupid."
I'm probably working on a scratch-build rocket or on a PCB