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\$
  • 1
    \$\begingroup\$ c should be volatile, declare it as: volatile uint8_t c=0; \$\endgroup\$ – BeB00 Jun 25 '18 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\$ – Atmega 328 Jun 25 '18 at 9:40
  • 2
    \$\begingroup\$ undefined behaviour will do that to you, \$\endgroup\$ – Jasen Jun 25 '18 at 9:57
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\$
  • \$\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\$ – Atmega 328 Jun 25 '18 at 10:14
  • \$\begingroup\$ Of course it can see C=1 there. But 1 is still less than 30, so... \$\endgroup\$ – Brian Drummond Jun 25 '18 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\$ – Brian Drummond Jun 25 '18 at 10:24
  • \$\begingroup\$ actually, it loads c into sRam anyway. The code using volatile actually use 2 bytes less flash memory. Strange! \$\endgroup\$ – Atmega 328 Jun 25 '18 at 10:48
  • \$\begingroup\$ Not so strange, and a good lesson in the perils of "premature optimisation". \$\endgroup\$ – Brian Drummond Jun 25 '18 at 10:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.