#2750 Front-end expression evaluation bug

[bsccara]

SDCC version: 3.7.1 #10406

Command line used:

--nooverlay --nogcse --nolabelopt --noinvariant --noinduction --noloopreverse --no-peep --no-reg-params --nolospre --use-non-free --opt-code-size libc18f.lib -c -mpic16 -p18f13k50 teste.c

Code to reproduce the bug:

#include <pic18fregs.h>

const unsigned short seg_a = 0b1101011111101101;
const unsigned short seg_b = 0b0010011110011111;

void set_bits(unsigned char sn)
{
    LATCbits.LATC0 = ((seg_a >> sn) & 1) ^ ~0x1; /* 1 */
    LATCbits.LATC1 = ((seg_b >> sn) & 1) ^ ~0x1; /* 2 */
    //LATCbits.LATC1 = ((seg_a >> sn) & 1); /* 3 */
    //LATCbits.LATC1 = ((seg_b >> sn) & 1); /* 4 */
    //LATCbits.LATC2 = ((seg_b >> sn) & 1) ^ (~0x1 & 1); /* 5 */
}

void main(void)
{
    unsigned char i;

    for (i = 0; i < 10; i++) set_bits(i);
}

Bug:
The expressions 1 and 2, clearly variant, are treated by the front-end as invariant and compiled to the following assembler:

;   .line   8; teste.c  LATCbits.LATC0 = ((seg_a >> sn) & 1) ^ ~0x1; /* 1 */
    BSF _LATCbits, 0
;   .line   9; teste.c  LATCbits.LATC1 = ((seg_b >> sn) & 1) ^ ~0x1; /* 2 */
    BSF _LATCbits, 1

If expression 2 is commented out and expression 3 is uncommented (expression 3 is a portion of expression 1) the code is compiled as:

;   .line   8; teste.c  LATCbits.LATC0 = ((seg_a >> sn) & 1) ^ ~0x1; /* 1 */
    MOVLW   LOW(_seg_a)
    MOVWF   TBLPTRL
    MOVLW   HIGH(_seg_a)
    MOVWF   TBLPTRH
    MOVLW   UPPER(_seg_a)
    MOVWF   TBLPTRU
    TBLRD*+ 
    MOVFF   TABLAT, r0x01
    TBLRD*+ 
    MOVFF   TABLAT, r0x02
    MOVF    r0x00, W
    MOVWF   FSR0L
    MOVFF   r0x01, r0x00
    MOVFF   r0x02, r0x03
    MOVF    FSR0L, W
    BZ  _00107_DS_
    NEGF    WREG
    BCF STATUS, 0
_00108_DS_:
    RRCF    r0x03, F
    RRCF    r0x00, F
    ADDLW   0x01
    BNC _00108_DS_
_00107_DS_:
    MOVF    r0x00, W
    ANDLW   0x01
    MOVWF   r0x00
    BSF _LATCbits, 0
;   .line   10; teste.c LATCbits.LATC1 = ((seg_a >> sn) & 1); /* 3 */
    MOVF    r0x00, W
    ANDLW   0x01
    RLNCF   WREG, W
    MOVWF   PRODH
    MOVF    _LATCbits, W
    ANDLW   0xfd
    IORWF   PRODH, W
    MOVWF   _LATCbits

If expressions 1 and 5 are the ones uncommented then the following is generated:

;   .line   8; teste.c  LATCbits.LATC0 = ((seg_a >> sn) & 1) ^ ~0x1; /* 1 */
    BSF _LATCbits, 0
;   .line   12; teste.c LATCbits.LATC2 = ((seg_b >> sn) & 1) ^ (~0x1 & 1); /* 5 */
    MOVLW   LOW(_seg_b)
    MOVWF   TBLPTRL
    MOVLW   HIGH(_seg_b)
    MOVWF   TBLPTRH
    MOVLW   UPPER(_seg_b)
    MOVWF   TBLPTRU
    TBLRD*+ 
    MOVFF   TABLAT, r0x01
    TBLRD*+ 
    MOVFF   TABLAT, r0x02
    MOVF    r0x00, W
    MOVWF   FSR0L
    MOVFF   r0x01, r0x00
    MOVFF   r0x02, r0x03
    MOVF    FSR0L, W
    BZ  _00107_DS_
    NEGF    WREG
    BCF STATUS, 0
_00108_DS_:
    RRCF    r0x03, F
    RRCF    r0x00, F
    ADDLW   0x01
    BNC _00108_DS_
_00107_DS_:
    MOVF    r0x00, W
    ANDLW   0x01
    MOVWF   r0x00
    MOVF    r0x00, W
    ANDLW   0x01
    RLNCF   WREG, W
    RLNCF   WREG, W
    MOVWF   PRODH
    MOVF    _LATCbits, W
    ANDLW   0xfb
    IORWF   PRODH, W
    MOVWF   _LATCbits

So there is clearly a problem determining the expression types, causing them to be treated as a invariants, depending on the surrounding code. I think the problem is on the front-end because if the --debug-xtra switch is added the following is generated (notice the '..AOP_LIT..' bit):

; ;ic       ` 0x160 (ADDRESS_OF)
; ;ic       = 0x3d  (=)
; ;ic       = 0x3d  (=)
; ; *** genNearPointerSet  9873
; ; pic16_aopOp 981
; ; *** 1032: symbol name = iTemp2, regType = 2
; ; 1048
; ; gen.c:796 dir size: 1
; ; 1686 pic16_popRegFromString _LATCbits 1/0
; ; 805: rname _LATCbits, val 0, const = 0
; ; *** genNearPointerSet  9891
; ; pic16_aopOp 981
; ; 1368: pic16_aopGet AOP_PCODE type PO_DIR
; ; _LATCbits offset 0
; ;     line = 9893 result AOP_PCODE=_LATCbits, left -=-, right AOP_LIT=0x01, size = 1
; ; *** genPackBits  9547
; ; genPackBits 9561 optimize bit assignment
; ; pic16_popGet AOP_PCODE (PO_DIR) 1892 _LATCbits offset 0
;   .line   8; teste.c  LATCbits.LATC0 = ((seg_a >> sn) & 1) ^ ~0x1; /* 1 */
    BSF _LATCbits, 0    ;key=008, flow seq=007

[bsccara]

Some more info:
By using the --dump-i-code switch and compiling the expressions 1 and 5 i got the .dumpraw1 file, which seems to pinpoint the problem (lines in bold below):

teste.c(l8)     iTemp1 {volatile-struct __00000200 near* fixed} = &[_LATCbits {volatile-struct __00000200 fixed} , 0x0 {const-unsigned-char literal}]
teste.c(l8)     iTemp2 {unsigned-bitfield {0,1} near* fixed} = iTemp1 {volatile-struct __00000200 near* fixed} + 0x0 {const-unsigned-char literal}
teste.c(l8)     iTemp3 {const-unsigned-int fixed} = @[_seg_a {const-unsigned-int code} + 0x0 {const-unsigned-char literal}]
teste.c(l8)     iTemp4 {const-unsigned-int fixed} = iTemp3 {const-unsigned-int fixed} >> iTemp0 {unsigned-char fixed}{ sir@ _set_bits_sn_1_1}
teste.c(l8)     iTemp5 {bit fixed} = gabit iTemp4 {const-unsigned-int fixed}
**teste.c(l8)       iTemp6 {bit fixed} = iTemp5 {bit fixed} | 0x1 {const-bit literal}**
teste.c(l8)     iTemp7 {unsigned-bitfield {0,1} fixed} = (unsigned-bitfield {0,1} fixed)iTemp6 {bit fixed}
teste.c(l8)     *(iTemp2 {unsigned-bitfield {0,1} near* fixed}) := iTemp7 {unsigned-bitfield {0,1} fixed}

teste.c(l12)        iTemp8 {volatile-struct __00000200 near* fixed} = &[_LATCbits {volatile-struct __00000200 fixed} , 0x0 {const-unsigned-char literal}]
teste.c(l12)        iTemp9 {unsigned-bitfield {2,1} near* fixed} = iTemp8 {volatile-struct __00000200 near* fixed} + 0x0 {const-unsigned-char literal}
teste.c(l12)        iTemp10 {const-unsigned-int fixed} = @[_seg_b {const-unsigned-int code} + 0x0 {const-unsigned-char literal}]
teste.c(l12)        iTemp11 {const-unsigned-int fixed} = iTemp10 {const-unsigned-int fixed} >> iTemp0 {unsigned-char fixed}{ sir@ _set_bits_sn_1_1}
teste.c(l12)        iTemp12 {bit fixed} = gabit iTemp11 {const-unsigned-int fixed}
**teste.c(l12)      iTemp13 {bit fixed} = iTemp12 {bit fixed} ^ 0x0 {const-bit literal}**
teste.c(l12)        iTemp14 {unsigned-bitfield {2,1} fixed} = (unsigned-bitfield {2,1} fixed)iTemp13 {bit fixed}
teste.c(l12)        *(iTemp9 {unsigned-bitfield {2,1} near* fixed}) := iTemp14 {unsigned-bitfield {2,1} fixed}

Apparently the XOR and one's complement operators are being morphed together into an OR operator on expression 1, which turns it into an invariant. The question is why is this happening...

[bsccara]

More info, AST dumped:

FUNCTION (_set_bits=0x1322b10) type (void fixed) args (unsigned-char fixed)
tree (0x1322a70) not decorated
(null):0:{ L1 B2
(null):0:  DECLARE SYMBOL (L1 B1 sn=0x131ffd0) type (unsigned-char fixed)
teste.c:8:  ASSIGN(=) (0x131e050) type (unsigned-bitfield {0,1} data)
teste.c:8:    PTR_ACCESS (0x131d0f0) type (unsigned-bitfield {0,1} data)
teste.c:8:      ADDRESS_OF (0x131d050) type (volatile-struct __00000200 near* fixed)
teste.c:8:        SYMBOL (L0 B0 LATCbits=0x131c970 @ 0x1145890) type (volatile-struct __00000200 fixed)
teste.c:8:      SYMBOL (L1 B2 LATC0=0x131cfb0 @ 0x131cc60)
teste.c:8:    OR (0x131dfb0) type (bit fixed)
teste.c:0:      GETABIT (0x1320740) type (bit fixed)
teste.c:8:        RIGHT_SHIFT (0x131d970) type (const-unsigned-int code)
teste.c:8:          SYMBOL (L0 B0 seg_a=0x131d4e0 @ 0x131a8d0) type (const-unsigned-int code)
teste.c:8:          SYMBOL (L1 B1 sn=0x131d8d0 @ 0x131be20) type (unsigned-char fixed)
teste.c:0:        CONSTANT (0x13206a0) value = 0, 0x0, 0.000000 type (const-unsigned-char literal)
teste.c:0:      CONSTANT (0x13213b0) value = 1, 0x1, 1.000000 type (const-bit literal)
teste.c:12:  ASSIGN(=) (0x131fdf0) type (unsigned-bitfield {2,1} data)
teste.c:12:    PTR_ACCESS (0x131ebc0) type (unsigned-bitfield {2,1} data)
teste.c:12:      ADDRESS_OF (0x131eb20) type (volatile-struct __00000200 near* fixed)
teste.c:12:        SYMBOL (L0 B0 LATCbits=0x131e440 @ 0x1145890) type (volatile-struct __00000200 fixed)
teste.c:12:      SYMBOL (L1 B2 LATC2=0x131ea80 @ 0x131e730)
teste.c:12:    XOR (0x131fd50) type (bit fixed)
teste.c:0:      GETABIT (0x1321950) type (bit fixed)
teste.c:12:        RIGHT_SHIFT (0x131f440) type (const-unsigned-int code)
teste.c:12:          SYMBOL (L0 B0 seg_b=0x131efb0 @ 0x131b0f0) type (const-unsigned-int code)
teste.c:12:          SYMBOL (L1 B1 sn=0x131f3a0 @ 0x131be20) type (unsigned-char fixed)
teste.c:0:        CONSTANT (0x13218b0) value = 0, 0x0, 0.000000 type (const-unsigned-char literal)
teste.c:0:      CONSTANT (0x1322840) value = 0, 0x0, 0.000000 type (const-bit literal)
(null):0:}

Line 8 has an OR token, while line 12 has a XOR one. Both should have XORs.

[Philipp Klaus Krause]

I verified that this is a front-end bug, affecting all targets.

Philipp

[Philipp Klaus Krause]

In [r10453], I added a test for this, which can be enabled by removing the define guard.

Philipp

[Philipp Klaus Krause]

What happens is that at some time, the result of ((seg_a >> sn) & 1) is considered to be _Bool / __bit. That value is then xored with something that is neither 0 nor 1, resulting in a 1 (and an optimization for some reason uses | to implement the 1, instead of plain assignment).

Philipp

[Erik Petrich]

Fixed in [r11013]