#798 critical does not disable interrupts on 8051

closed-rejected
nobody
None
5
2013-05-25
2004-08-17
Anonymous
No

Problem
----------
8051 code produced by "critical" does not disable
interrupts. EA is tested and its state is saved on the
stack and restored at the end. However, EA is never set
to 0.

Sample code (main.c)
--------------------------
void main (void)
{
volatile int x;

while (1)
{
x +=1;
critical
{
x = 2;
}
}
}

Command Line
------------------
sdcc main.c

Output Code
---------------
128 ;main.c:5: while (1)
0031 129 00102$:
130 ;main.c:7: x +=1;
131 ; genPlus
132 ; genPlusIncr
0031 05 08 133 inc _main_x_1_1
0033 E4 134 clr a
0034 B5 08 02 135 cjne a,_main_x_1_1,00107$
0037 05 09 136 inc (_main_x_1_1 + 1)
0039 137 00107$:
138 ;main.c:11: }
139 ; genCritical
0039 D3 140 setb c
003A 10 AF 01 141 jbc ea,00108$
003D C3 142 clr c
003E 143 00108$:
003E C0 D0 144 push psw
145 ;main.c:10: x = 2;
146 ; genAssign
0040 E4 147 clr a
0041 F5 09 148 mov (_main_x_1_1 + 1),a
0043 75 08 02 149 mov _main_x_1_1,#0x02
150 ; genEndCritical
0046 D0 D0 151 pop psw
0048 92 AF 152 mov ea,c
153 ; Peephole 112.b
changed ljmp to sjmp
004A 80 E5 154 sjmp 00102$

E-mail
---------
You may contact me at bgoodman2@cyclos.com

Bill Goodman

Discussion

  • Logged In: NO

    Ooops. I meant to give the SDCC version:
    2.4.3 #791 (Aug 6 2004) (MINGW32)

     
  • Logged In: NO

    Ignore this bug report.

    I failed to fully comprehend the effect of the JBC instruction.

    Sorry for the false alarm.

    Bill Goodman
    bgoodman2@cyclos.com

     
  • Erik Petrich
    Erik Petrich
    2004-08-18

    • milestone: --> non_bugs
    • status: open --> closed-rejected
     
  • Logged In: YES
    user_id=589052

    Hi,

    the following (slightly shorter/quicker) code should
    do for critical functions as well:

    mov C,_EA
    push psw
    clr _EA
    ...
    pop psw
    mov _EA,C

    Shouldn't we use this instead or am I missing something?

    Frieder

     
  • Maarten Brock
    Maarten Brock
    2004-08-18

    Logged In: YES
    user_id=888171

    Frieder,

    This was the old code and I requested it to be changed. JBC
    is an atomical action, your proposal isn't. I argue to use
    atomical actions on (interrupt) flags as often as possible.
    What if for instance an interrupt occurs between mov C,_EA
    and clr _EA and the interrupt turns _EA on or off? It gets set
    to an old value after the critical section.

    Now it doesn't have to set C and push psw. An alternative
    could be to insert a new local bit variable to save _EA. But
    that better wait untill SDCC can handle bit variables a "bit"
    better.

    Maarten

     
  • Logged In: YES
    user_id=589052

    Hi Maarten,
    I do only see one possibly problematic situation:

    Interrupts are enabled and the interrupt occurs
    at the place you mentioned and the interrupt
    routine disables the _global_ interrupt enable.

    While it is not unusual for an interrupt to
    switch off its interrupt source disabling the global
    interrupt enable would seem very unusual to me.
    (if you want to catch this case you need the code
    with the jbc, we do agree there)
    So to my opinion this boils down to the question:
    is any code known which switches off global IRQ
    enable during the interrupt routine itself? Say yes
    and my proposal is obsolete;)

    Thx
    Frieder

     
  • Maarten Brock
    Maarten Brock
    2004-08-19

    Logged In: YES
    user_id=888171

    Hello Frieder,

    I don't think this is about what's (un)usual, but to guarantee
    something will work. Furthermore, your proposal is only 1 byte
    shorter. Don't know about speed, but that's different for
    many derivatives and therefor hard to optimize.

    And in my mind there is still an RFE to enhance critical with
    an optional parameter to select the flag. Critical doesn't
    always need all interrupts disabled, sometimes we can do
    with only disabling a specific interrupt. Something like "critical
    (ES)".

    Can we please first concentrate on making SDCC bug free
    and worry about optimizations later. Especially minor ones like
    these? No offence intended.

    Greets,
    Maarten