#3706 MSC51 incorrect code Intermittently generated for 16 bit decrement

[Skip Hansen]

For the C source

   int CmdBytes;
...
   CmdBytes = CmdBytes - 1;

This is generated

                                   2950 ;   main.c:490: CmdBytes = CmdBytes - 1;
                                   2951 ;   genMinus
                                   2952 ;   genMinusDec
      0007E4 EB               [12] 2953     mov a,r3
      0007E5 24 FF            [12] 2954     add a,#0xff
      0007E7 FA               [12] 2955     mov r2,a
      0007E8 EA               [12] 2956     mov a,r2
      0007E9 34 FF            [12] 2957     addc    a,#0xff
      0007EB FB               [12] 2958     mov r3,a

As a result when CmdBytes is 0x0005 the result of the decrement is 0x0404.

SDCC versions 4.2.0, 4.3.0 and 4.4.0-rc3 all show the same bug.

I have been trying to simplify the code, but the bug is dependent on the registers assigned CmdBytes. Some minor edits cause CmdBytes to be assigned to a different set of registers for which correct code is generated. Other edits cause the source and destination register sets to be different and again correct code is generated.

All SDCC versions were built from sources on Ubuntu 20.04.1.

The command line to build main.c was:

skip@Dell-7040:~/esl/dmitrygr-einkTags/firmware/proxy$ ./build_main.sh
++ '[' '!' -e build ']'
++ mkdir build
++ sdcc -c main.c -I../board/chroma29r -I../soc/cc111x -I../8051 -I../../common -I../common -I../cpu/8051 -I. --code-size 0x7f80 --xram-loc 0xf000 --xram-size 0xda2 --model-medium -mmcs51 --std-c2x --opt-code-size --fomit-frame-pointer -DNO_AUTO_UART_SWITCH -DBUILD_TYPE=chroma29r -MD -DPROXY_BUILD -I. --debug --fverbose-asm --no-peep -o build/main.rel
main.c:153: warning 110: conditional flow changed by optimizer: so said EVELYN the modified DOG

Please find attached main.c, main.lst plus the complete project source.

The script build_main.sh compiles just main.c.

I've also inluded the scripts that I used to compile SDCC in the sdcc directory.

The project is open source and is here if anyone is interested: https://github.com/skiphansen/dmitrygr-einkTags

[Konstantin Kim]

reduced code:

void foo(int);
extern __xdata char* gRxBuf;

void EpdCmd(void) {
    char __xdata *pData = gRxBuf;
    int MsgLen = 8;
    int CmdBytes;

    do {
        CmdBytes  = *pData++;
        MsgLen   -= CmdBytes;
        CmdBytes -= 1;  // <-- bug
        foo(CmdBytes);
        pData++;
    } while(MsgLen) ;
}

command line: sdcc --model-medium -mmcs51 --no-peep -c test.c

                                164 ; main.c:14: CmdBytes -= 1;   // <-- bug
                                165 ; genMinus
                                166 ; genMinusDec
  00002C EC               [12]  167   mov a,r4
  00002D 24 FF            [12]  168   add a,#0xff
  00002F FB               [12]  169   mov r3,a
  000030 EB               [12]  170   mov a,r3
  000031 34 FF            [12]  171   addc    a,#0xff
  000033 FC               [12]  172   mov r4,a

[RvS]

Just an observation:
When compiling with --i-code-in-asm the i-code reveals that the order of r3 r4 is different for iTemp2 and iTemp13. This looks odd to me. Any idea why that is different?

;   test.c:12: CmdBytes -= 1;  // <-- bug
;   [--2345--] ic:13:   iTemp2 [k7 lr11:12 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{int fixed}{ sir@ _EpdCmd_CmdBytes_10000_3}[**r3 r4** ] = iTemp13 [k21 lr9:11 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{int fixed}[**r4 r3** ] - 0x1 {const-int literal}
    mov a,r4
    add a,#0xff
    mov r3,a
    mov a,r3
    addc    a,#0xff
    mov r4,a
;   test.c:13: foo(CmdBytes);
;   [--2345--] ic:15:   send iTemp2 [k7 lr11:12 so:0]{ ia0 a2p0 re1 rm0 nos0 ru0 dp0}{int fixed}{ sir@ _EpdCmd_CmdBytes_10000_3}[r3 r4 ]{argreg = 1}
;   [--2--5--] ic:16:   iTemp9 [k17 lr13:13 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{void fixed} = call _foo [k15 lr0:0 so:0]{ ia0 a2p0 re0 rm0 nos0 ru0 dp0}{void function ( int fixed) fixed}

[d00f]

I think this one is fixed by [r17025] (bug [#4153]). The reduced test case from @kimstig shows the same pattern I ran into there: the result of the 16-bit subtract lands in the same registers as the operand, but in swapped order, so mov r3,a clobbers the high byte before addc reads it.

With a clean 4.6.0 and --model-medium, with or without --no-peep, I still get:

mov a,r4
add a,#0xff
mov r3,a
addc a,#0xff
mov r4,a

With 4.6.0 plus only the positionRegs() change from [#4153], and with trunk [r17039], the same code becomes:

dec r3
cjne r3,#0xff,00114$
dec r4

The spillThis() fix from [#4154] isn't needed for this case. Maybe this can be closed as a duplicate once someone confirms on their side.

[Skip Hansen]

Thank you, I'll test it later this week and report back.

[Skip Hansen]

I just tested my project with

skip@Dell-5510:~/esl/Tag_FW_Chroma/Chroma_Tag_FW/OEPL$ sdcc --version
SDCC : mcs51/r4k/r5k/r6k/ez80/r800/huc6280/f8/f8l TD- 4.6.4 #17068 (Linux)
published under GNU General Public License (GPL)

And it worked perfectly!

And additionally the finaly binary is 591 bytes smaller than the same code compiled with 4.2.0.
The code size is very important to me since the lack of flash space has prevented me from adding features.

As far as I'm concerned this bug can be closed

THANK YOU SCDCC TEAM !

[Maarten Brock]

Added test to regression test bug-4153.c.