#2827 bitwise assignment operator regression

[Jens]

In recent versions of sdcc (somewhere after 3.6.3 #9771), bitwise assignment operations on 8051 port latches are generating multiple instructions, e.g. read from port with a mov to temp register, operating on this register, then mov'ing it back to latch. Previously this would be done with one logical bitwise operation (anl, orl, xrl). Looks like 8051 port latches are special in that, reading from them (e.g. mov) gives you the input values (not the output values), while the anl, orl, xrl, do a read-modify-write operation on the output values.

expected behavior:
Bitwise assignment (&=, |=, ^=) should be using anl, orl, and xrl instructions
ref:
http://www.keil.com/support/docs/2442.htm
https://www.silabs.com/community/mcu/8-bit/knowledge-base.entry.html/2013/11/25/port_latch_read-modi-1MBJ

examples:
previous (good) version of sdcc (3.6.3 #9771):

;   src/main.c:175: LED_DIGITS_PORT &= ~((1<<LED_DIGITS_PORT_BASE) << digit);
    mov b,r6
    inc b
    mov a,#0x04
    sjmp    00223$
00221$:
    add a,acc
00223$:
    djnz    b,00221$
    cpl a
    mov r6,a
    anl _P3,a

newer (bad) version of sdcc (sdcc 3.8.1, #10620):

;   src/main.c:175: LED_DIGITS_PORT &= ~((1<<LED_DIGITS_PORT_BASE) << digit);
    mov b,r6
    inc b
    mov a,#0x04
    sjmp    00246$
00244$:
    add a,acc
00246$:
    djnz    b,00244$
    cpl a
    mov r6,a
    mov r5,_P3
    anl a,r5
    mov _P3,a

[Jens]

clarification of expected behavior statement:
Bitwise assignment (&=, |=, ^=) should be using anl, orl, and xrl instructions directly on the target port latch (or should this be any SFR in general?)

[john munch]

It should be all Sfr's!
EIE2 |= E2_MAT; // enable port match interrupt
now compiles for MCS51 as:
mov r6,_EIE2
mov r7,#0x00
orl ar6,#0x02
mov _EIE2,r6
instead of:
orl _EIE2,#0x02

which not only makes it less efficient but also makes it non-atomic which can be a major problem when enabling/disabling interrupts in interrupt service routines.

All this caused some of my boards to hang after a few hours when I upgraded. Yes, there are workarounds. I chose to change 0x02 to (unsugned char)0x02 in the define of E2_MAT and that fixes it. 0x02&0xff also fixes it.

[Erik Petrich]

This looks like the same root cause as [#2877] and [#2733]. The normal integer promotion rules casts P3 to int to apply the bitwise-AND, then later optimization determines that only an 8-bit result is needed and changes the cast to signed char. But the sfr type is considered unsigned so the cast is not removed, which prevents the mcs51 backend from using the atomic form of anl or orl.

[Erik Petrich]

A workaround for this specific problem is in revision [r11618]. Still looking at how to prevent the type from changing from unsigned char to signed int to signed char rather than back to unsigned char.