This discussion has been locked.
You can no longer post new replies to this discussion. If you have a question you can start a new discussion

Overkeen Optimisation of Volatile variable

I have ported some code to an 8051 based processor using Keil C51 v7.06 (from Cosmic for ST7).

The code worked fine with Cosmic, and indeed nearly all of it works with C51. One area is causing trouble. The original code used a global counter value that was incremented by a timer interrupt routine. This was then used for things like timing key press events to determine long or short key press times and take appropriate action. The code is as follows:

-----module1.c------

volatile unsigned char rtc_count;

/*
** Timer 1 interrupt, every 25ms
**  Used for other things too - but simplified for posting
*/
void RTC_int(void) interrupt 3
{
    ++rtc_count;
}

-----module2.c------
extern volatile unsigned char rtc_count;

   rtc_count = 0;

   while (read_keys() == TEST) {
      if (rtc_count >= 80) {
          // 2 seconds elapsed
    	    break;
      }
   }

   if  (rtc_count >= 80) {
        // do long keypress action
   }
   else {
        // do short keypress actions
   }
The trouble appears to be that the volatile is NOT recognised. Holding the key will not result in the break statement ever happening.

If I change the while loop to include a statement that actually uses the rtc_value, such as:

   while (read_keys() == TEST) {
      printf("%u%, (unsigned int)rtc_count);
      if (rtc_count >= 80) {
          // 2 seconds elapsed
    	    break;
      }
   }

Then the 2 second keypress works every time.

The listing file for the first piece of code is as follows:
0013 E4                CLR     A
0014 900000      E     MOV     DPTR,#rtc_count
0017 F0                MOVX    @DPTR,A
                                           ; SOURCE LINE # 811
0018 900000      E     MOV     DPTR,#sysinfo
001B E0                MOVX    A,@DPTR
001C FF                MOV     R7,A
001D 13                RRC     A
001E 13                RRC     A
001F 543F              ANL     A,#03FH
0021 20E00F            JB      ACC.0,?C0138
0024         ?C0139:
                                           ; SOURCE LINE # 812
0024 120000      E     LCALL   read_sr
0027 BF4109            CJNE    R7,#041H,?C0138
                                           ; SOURCE LINE # 821
002A 900000      E     MOV     DPTR,#rtc_count
002D E0                MOVX    A,@DPTR
002E C3                CLR     C
002F 9450              SUBB    A,#050H
0031 40F1              JC      ?C0139
                                           ; SOURCE LINE # 823
0033         ?C0138:


If I add the printf statement (actually an sprintf, and call to LCD output routine) the lisitng code is as follows:

0013 E4                CLR     A
0014 900000      E     MOV     DPTR,#rtc_count
0017 F0                MOVX    @DPTR,A
                                           ; SOURCE LINE # 811
0018 900000      E     MOV     DPTR,#sysinfo
001B E0                MOVX    A,@DPTR
001C FF                MOV     R7,A
001D 13                RRC     A
001E 13                RRC     A
001F 543F              ANL     A,#03FH
0021 20E044            JB      ACC.0,?C0138
0024         ?C0139:
                                           ; SOURCE LINE # 812
0024 120000      E     LCALL   read_sr
0027 EF                MOV     A,R7
0028 6441              XRL     A,#041H
002A 703C              JNZ     ?C0138
                                           ; SOURCE LINE # 818
002C 7BFF              MOV     R3,#0FFH
002E 7A00        R     MOV     R2,#HIGH ?SC_129
0030 7900        R     MOV     R1,#LOW ?SC_129
0032 900000      E     MOV     DPTR,#?_sprintf?BYTE+03H
0035 120000      E     LCALL   ?C?PSTXDATA
0038 900000      E     MOV     DPTR,#rtc_count
003B E0                MOVX    A,@DPTR
003C FF                MOV     R7,A
003D 900000      E     MOV     DPTR,#?_sprintf?BYTE+06H
0040 E4                CLR     A
0041 F0                MOVX    @DPTR,A
0042 A3                INC     DPTR
0043 EF                MOV     A,R7
0044 F0                MOVX    @DPTR,A
0045 7B01              MOV     R3,#01H
0047 7A00        E     MOV     R2,#HIGH message
0049 7900        E     MOV     R1,#LOW message
004B 120000      E     LCALL   _sprintf
                                           ; SOURCE LINE # 819
004E 7B01              MOV     R3,#01H
0050 7A00        E     MOV     R2,#HIGH message
0052 7900        E     MOV     R1,#LOW message
0054 900000      E     MOV     DPTR,#?_PrintLCDString?BYTE+04H
0057 7401              MOV     A,#01H
0059 F0                MOVX    @DPTR,A
005A 7D02              MOV     R5,#02H
005C 120000      E     LCALL   _PrintLCDString
                                           ; SOURCE LINE # 821
005F 900000      E     MOV     DPTR,#rtc_count
0062 E0                MOVX    A,@DPTR
0063 C3                CLR     C
0064 9450              SUBB    A,#050H
0066 40BC              JC      ?C0139
                                           ; SOURCE LINE # 823
0068         ?C0138:


Optimisation is set to level 8, and the memory model is LARGE. I cannot easily change both as we are nearing the memory capacity of the device, and it overflows (I guess I can cut out some code and try it, but I really shouldn't have to do that).

In both cases of the above, the last few lines clearly show the value of the counter is being read and tested. But in the latter the long keypress works, and I can see the value of the rtc_count increment to 80 on the latter too (so it is not being reset by something else - there are no other changes to the code!).

Most confounding.

I have tried changing the test from the simple "if (var > const)" to "if (func() > const)" with func() returning the rtc_count variable, and that does not work either.

I would be grateful for any constructive suggestions. As I mentioned at the top, this will compile and work on a Cosmic compiler.

Thankyou
Gary

Parents
  • There is no difference in the assembly for the test of rtc_count. While the volatility is a good guess at a possible cause, it doesn't seem to be the case here.

    Program 1:
    
    002A 900000      E     MOV     DPTR,#rtc_count
    002D E0                MOVX    A,@DPTR
    002E C3                CLR     C
    002F 9450              SUBB    A,#050H
    0031 40F1              JC      ?C0139
    
    

    Program 2:
                                               ; SOURCE LINE # 821
    005F 900000      E     MOV     DPTR,#rtc_count
    0062 E0                MOVX    A,@DPTR
    0063 C3                CLR     C
    0064 9450              SUBB    A,#050H
    0066 40BC              JC      ?C0139
    
    

    The value of rtc_count is being loaded and tested on every loop.

    The difference I do notice is in the while loop:

    Program 1:
    
    0024 120000      E     LCALL   read_sr
    0027 BF4109            CJNE    R7,#041H,?C0138
    

    Program 2:
                                               ; SOURCE LINE # 812
    0024 120000      E     LCALL   read_sr
    0027 EF                MOV     A,R7
    0028 6441              XRL     A,#041H
    002A 703C              JNZ     ?C0138
    

    I'm not sure why the second case doesn't collapse to a CJNE R7,#41h,?CO138 like the first. But they seem functionally equivalent to me.

    The other difference that strikes me is that the printf() call is big and slow. How does your read_keys() (read_sr) routine work? What does it return if you sample the keyboard back to back very quickly? Since your code is trying to "loop while key held down", if you get some sort of null result from read_keys(), it would break out of the loop as "none" wasn't the A key. Perhaps the printf() slows things down enough to make the keyboard sampling work. (And perhaps the Cosmic compiler is just slow enough overall to have the same effect!)

Reply
  • There is no difference in the assembly for the test of rtc_count. While the volatility is a good guess at a possible cause, it doesn't seem to be the case here.

    Program 1:
    
    002A 900000      E     MOV     DPTR,#rtc_count
    002D E0                MOVX    A,@DPTR
    002E C3                CLR     C
    002F 9450              SUBB    A,#050H
    0031 40F1              JC      ?C0139
    
    

    Program 2:
                                               ; SOURCE LINE # 821
    005F 900000      E     MOV     DPTR,#rtc_count
    0062 E0                MOVX    A,@DPTR
    0063 C3                CLR     C
    0064 9450              SUBB    A,#050H
    0066 40BC              JC      ?C0139
    
    

    The value of rtc_count is being loaded and tested on every loop.

    The difference I do notice is in the while loop:

    Program 1:
    
    0024 120000      E     LCALL   read_sr
    0027 BF4109            CJNE    R7,#041H,?C0138
    

    Program 2:
                                               ; SOURCE LINE # 812
    0024 120000      E     LCALL   read_sr
    0027 EF                MOV     A,R7
    0028 6441              XRL     A,#041H
    002A 703C              JNZ     ?C0138
    

    I'm not sure why the second case doesn't collapse to a CJNE R7,#41h,?CO138 like the first. But they seem functionally equivalent to me.

    The other difference that strikes me is that the printf() call is big and slow. How does your read_keys() (read_sr) routine work? What does it return if you sample the keyboard back to back very quickly? Since your code is trying to "loop while key held down", if you get some sort of null result from read_keys(), it would break out of the loop as "none" wasn't the A key. Perhaps the printf() slows things down enough to make the keyboard sampling work. (And perhaps the Cosmic compiler is just slow enough overall to have the same effect!)

Children
  • Thanks for your comments Drew.

    I have been trying other things, and as you say the volatile variable was my prime suspect. I even upgraded my compiler to v7.10, without any effect.

    Your comments on the null result and speed of operation got me to change tacts.

    The other difference that strikes me is that the printf() call is big and slow. How does your read_keys() (read_sr) routine work?

    Investigating the read_keys() routine [ as you quite rightly spotted I had changed the C code to make it a little more obvious - it is read_sr() ] with attention to the speed of operation I found that it was not always accurate when called as quickly as this. Adding a small delay to the calling loop - and hey presto it works perfectly. You hit the nail on the head with:

    Perhaps the printf() slows things down enough to make the keyboard sampling work. (And perhaps the Cosmic compiler is just slow enough overall to have the same effect!)

    My latest, updated version is now a clean and simple

        rtc_count = 0;
        if (!sysinfo.testmode) {
            while ((read_sr() == TEST) && (rtc_count < 80)) {
                delay_ms(30);
            }
        }
    

    I have yet to go back and see quite what the Cosmic version was doing differently - the hardware is eactly the same other than the change to a new micro, hence pointing the finger at the operation of the micro + software.

    Easy to get hung up on something and blame the wrong bit. I owe you a beer!

    Thanks,
    Gary

  • Hallo ALL,
    I think I have the same problem ....
    I use F005 from cygnal.
    I have a timer:
    ......................................
    idata U8 volatile HeartbeatLEDTimer;
    idata U8 volatile DACTimer;
    ...................................
    void Timer0InterruptHandler(void) interrupt 1 using 1
    {
    // Reload the timer registers

    TR0 = 0;

    TL0 = T0_RELOAD_LOW;
    TH0 = T0_RELOAD_HIGH;



    // DECREMENT_TIMER(HeartbeatLEDTimer);
    DECREMENT_TIMER(DACTimer);

    TR0 = 1;

    }



    and in the main()
    /*
    if(!HeartbeatLEDTimer) {pin_LED_D1 = ~pin_LED_D1; HeartbeatLEDTimer = HEART_BEAT_LED_TIMER;}
    */
    .............................

    // INTERRUPTS_OFF;
    DACTimer=DAC_TIMER;
    // INTERRUPTS_ON;
    while(DACTimer != 0){; }//delay



    it works without INTERRUPTS_OFF if I don't use HeartbeatLEDTimer, but if I use it, I should disable interrupts before DACTimer=DAC_TIMER and not for HeartbeatLEDTimer = HEART_BEAT_LED_TIMER???
    I'm really confused and I thought the problem is in VOLATILE! But it's not so clear.
    Please help me to understand what here is really happened.
    With best regards,
    Sergey