0
\$\begingroup\$

It is strange and rare. I am trying to blink a LED at PD0 at 2hz, but it doesn't work. Seems to me that while loop doesn't work at all. I have triple checked the code. what is the problem here?

uint8_t c=0; 

int main(void){

DDRD= 1<<0 ;
TCCR0 = (1 << CS02) |  (1 << CS00); // (clk_i/o )/1024 ( Prescaler) 

TIMSK |= 1<< TOIE0;
sei();

  while (1){ 
  if ( c >=30 ) { PORTD ^= 1<<0; c=0; }                //61.035/30= 2.034 hz
  }

  }

ISR (TIMER0_OVF_vect){
  c++;   //16000000/1024/256=61.035 hz
  }  

Of course, I know the solution. It's by adding a random line of code to the while loop, and it works again. Strange!! What would be the problem? compiler bug??

uint8_t c=0; 

int main(void){

DDRD= 1<<0 ;
TCCR0 = (1 << CS02) |  (1 << CS00); // (clk_i/o )/1024 ( Prescaler) 

TIMSK |= 1<< TOIE0;
sei();

  while (1){ 
  if ( c >=30 ) { PORTD ^= 1<<0; c=0; }                //61.035/30= 2.034 hz
  _NOP();
  }

  }

ISR (TIMER0_OVF_vect){
  c++;   //16000000/1024/256=61.035 hz
  }  
\$\endgroup\$
3
  • 1
    \$\begingroup\$ c should be volatile, declare it as: volatile uint8_t c=0; \$\endgroup\$
    – BeB00
    Commented Jun 25, 2018 at 9:36
  • \$\begingroup\$ @BeB00 Yes, you are right, but why it still works even I don't declare it as volatile, when I added a random line of code to the while loop? \$\endgroup\$
    – user83582
    Commented Jun 25, 2018 at 9:40
  • 2
    \$\begingroup\$ undefined behaviour will do that to you, \$\endgroup\$ Commented Jun 25, 2018 at 9:57

1 Answer 1

5
\$\begingroup\$

By NOT declaring it volatile you give the compiler permission to optimise.

In the first loop, the compiler knows the value of C at all times, because nothing else in the loop can modify C (and it isn't volatile) so, because C is always 0, the loop doesn't need to do anything. The code is working perfectly because that's what you asked for.

In the second loop, you call a procedure (oops, void function) NOP() which could do absolutely anything. So, unless the compiler does whole program analysis over all the libraries (and it doesn't), how does it know if NOP() modifies C?

It doesn't, so it can't make any assumptions about C's value and has to re-fetch it from memory.

Ths second program isn't working, it does what you expect by accident.

If the "random additional line" didn't involve a call - e.g. it was an inline assembly NOP instruction, you would probably see different behaviour.

Declare C volatile.

\$\endgroup\$
5
  • \$\begingroup\$ In the first loop, changing c=0 to c=1 doesn't work either. Can't the compiler see c=1 there? Is there a way not to use volatile, because 1 byte of sRam is 1 byte. \$\endgroup\$
    – user83582
    Commented Jun 25, 2018 at 10:14
  • \$\begingroup\$ Of course it can see C=1 there. But 1 is still less than 30, so... \$\endgroup\$
    – user16324
    Commented Jun 25, 2018 at 10:21
  • \$\begingroup\$ Where else other than SRAM are you going to store c to share it between ISR and main loop? Alternatives : program the timer for the correct frequency, and you only need to store a boolean. I use Ada so it's trivial to pack 8 Booleans into a byte. In C it's a bit of a mess but you can do it if you have to. \$\endgroup\$
    – user16324
    Commented Jun 25, 2018 at 10:24
  • \$\begingroup\$ actually, it loads c into sRam anyway. The code using volatile actually use 2 bytes less flash memory. Strange! \$\endgroup\$
    – user83582
    Commented Jun 25, 2018 at 10:48
  • \$\begingroup\$ Not so strange, and a good lesson in the perils of "premature optimisation". \$\endgroup\$
    – user16324
    Commented Jun 25, 2018 at 10:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.