#2921 [mcs51] accumulator use not preserved

[Jens]

It appears any use of accumulator could be arbitrarily overwritten by compiler without warning.

I could not find any cautions or warnings in sdcc manual about use of accumulator sfr/sbit access from c code.

Expected behavior:
Compiler should detect ACC sfr/sbit access, and either:

1. allow accumulator sfr/sbit access, and preserve a when using it as temp var, or
2. emit warning or error telling user not to use ACC

there is a very old bug, #76, which seems to describe this issue

However, I bring up this bug, because I see several examples from Keil C51 code-land which use this pattern (direct c access to ACC), which implies they may allow/preserve a.
(I actually ran into this while porting some C51 code to sdcc.)
This also implies they allow C access to ACC sbits:
http://www.keil.com/support/docs/1934.htm

example code:

void ds1302InputByte(uint8_t value)
{ 
    uint8_t i;

    ACC = value;
    for (i = 8; i > 0; i--) {
        DS1302_IO = ACC0;
        DS1302_SCLK = 1;
        DS1302_SCLK = 0;
        ACC = ACC >> 1; 
    }

    return;
}

compiled to:

;------------------------------------------------------------
;Allocation info for local variables in function 'ds1302InputByte'
;------------------------------------------------------------
;value                     Allocated to registers 
;i                         Allocated to registers r7 
;------------------------------------------------------------
;       src/ds1302.c:46: void ds1302InputByte(uint8_t value)
;       -----------------------------------------
;        function ds1302InputByte
;       -----------------------------------------
_ds1302InputByte:
        mov     _ACC,dpl
;       src/ds1302.c:51: for (i = 8; i > 0; i--) {
        mov     r7,#0x08
00102$:
;       src/ds1302.c:52: DS1302_IO = ACC0;
;       assignBit
        mov     c,_ACC0
        mov     _P1_5,c
;       src/ds1302.c:53: DS1302_SCLK = 1;
;       assignBit
        setb    _P5_5
;       src/ds1302.c:54: DS1302_SCLK = 0;
;       assignBit
        clr     _P5_5
;       src/ds1302.c:55: ACC = ACC >> 1; 
        mov     a,_ACC
        clr     c
        rrc     a
        mov     _ACC,a
;       src/ds1302.c:51: for (i = 8; i > 0; i--) {
        mov     a,r7
        dec     a
        mov     r7,a
        jnz     00102$
;       src/ds1302.c:58: return;
;       src/ds1302.c:59: }
        ret
;------------------------------------------------------------

[Jens]

notice that acc is getting clobbered by the loop decrement, without preserving a. It's as if sdcc doesnt understand that ACC === a

also fwiw, earlier in asm, sbit access to ACC is declared:

_ACC    =       0x00e0

_ACC0   =       0x00e0
_ACC7   =       0x00e7

however, to be clear, this doesn't appear to have anything to do with the sbit access, but even access to the stock (defined in 8051.h) ACC sfr is not detected and preserved.

[Jens]

reproduced on both 3.8.4 and 3.9.0 (but looks like from old bugs, it's always been there)

the simplest example:

#include <8051.h>
#include <stdint.h>

uint8_t j = 0;

void foo(uint8_t b)
{
  uint8_t i;
  ACC = b;
  for (i=8; i > 0; i--)
  {
    P0_0 = ACC & 1;
    ACC = ACC >> 1;
  }
}

void main() 
{
  foo(0xaa);
}

;------------------------------------------------------------
;Allocation info for local variables in function 'foo'
;------------------------------------------------------------
;b                         Allocated to registers 
;i                         Allocated to registers r7 
;------------------------------------------------------------
;       test.c:6: void foo(uint8_t b)
;       -----------------------------------------
;        function foo
;       -----------------------------------------
_foo:
        ar7 = 0x07
        ar6 = 0x06
        ar5 = 0x05
        ar4 = 0x04
        ar3 = 0x03
        ar2 = 0x02
        ar1 = 0x01
        ar0 = 0x00
        mov     _ACC,dpl
;       test.c:10: for (i=8; i > 0; i--)
        mov     r7,#0x08
00102$:
;       test.c:12: P0_0 = ACC & 1;
        mov     a,_ACC
        anl     a,#0x01
        add     a,#0xff
        mov     _P0_0,c
;       test.c:13: ACC = ACC >> 1;
        mov     a,_ACC
        clr     c
        rrc     a
        mov     _ACC,a
;       test.c:10: for (i=8; i > 0; i--)
        mov     a,r7
        dec     a
        mov     r7,a
        jnz     00102$
;       test.c:15: }
        ret
;------------------------------------------------------------
:

[Jens]

sorry the old reference was not a bug but a support request:
https://sourceforge.net/p/sdcc/support-requests/76/

(it just has a comment by Maarten that accumulator access might get clobbered; but there is no other documentation reference or compiler warnings, which is what I'm looking for.
A compiler error which slaps me saying " hey stupid, don't use ACC directly" seems like the easiest option.)

[Maarten Brock]

IMHO it is plain stupid to expect that you can fiddle with the core registers from C without consequences. They are all the compiler can work with and it needs them badly. This includes ACC, B, C, DPTR, SP, R0-R7 and other register banks if used. The fact that the mcs51 is constructed such that you could does not mean that you should.

It is also not straightforward to detect a redefinition of such a register since uninitialized absolutely placed variables (__at) are overlayed. This makes giving an error difficult.

If you want to optimize this kind of routines, use (inline) assembly and write the whole loop yourself.

The 'bug' if you want is that this is not properly documented.

[Maarten Brock]

Allthough the latest version of SDCC still emits redundant mov a,_ACC, it now works since the loop decrement is now done witha djnz r7. By works I mean that it happens to work by accident.

[Oleg Endo]

I (ab)use this very peculiarity to pass variables from C code to inline asm code and vice-versa. So direct manipulation of compiler-used registers is actually useful, because SDCC lacks any other mean to exchange data between C and asm blocks (as far as I'm aware).

void example (uint16_t cmd)
{
  ACC = (cmd >> 8) & 0xFF;

  __asm
      rlc a
      clr _P1_6
      mov _P1_3,c
      setb _P1_6

      rlc a
      clr _P1_6
      mov _P1_3,c
      setb _P1_6

     ....
 __endasm;
}