#2258 PIC14 bit clear operations not optimized

open
nobody
None
PIC14
1
2014-10-29
2014-03-04
Visenri
No

Setting bits using defined bits generates correct code:

PORTCbits.RC0 = 1;
PORTCbits.RC0 = 0;

compiles to:

;   .line   11; "newmain.c" PORTCbits.RC0 = 1;
    BSF _PORTCbits,0
;   .line   12; "newmain.c" PORTCbits.RC0 = 0;
    BCF _PORTCbits,0

Setting bits in a portable way does not generate the expected result:

#define bit_set(var,bitno)   ((var) |=   1 << (bitno) )
#define bit_clear(var,bitno) ((var) &= ~(1 << (bitno)))

    bit_set(PORTC,0);
    bit_clear(PORTC,0);

compiles to:

;   .line   14; "newmain.c" bit_set(PORTC,0);
    BSF _PORTC,0
;   .line   15; "newmain.c" bit_clear(PORTC,0);
    MOVF    _PORTC,W
    BANKSEL r0x1000
    MOVWF   r0x1000
    MOVLW   0xfe
    ANDWF   r0x1000,W
    BANKSEL _PORTC
    MOVWF   _PORTC

As you can see, set bit operation is recognized correctly, but clear bit operation results in a long sequence anding the port with a temporal register and writing it back to the port.
This is far from optimal code and can cause problems if the same port is being used in the interrupt code.

command used:
sdcc.exe --use-non-free -c -mpic14 -p16f1824 newmain.c -onewmain.o

sdcc version:
sdcc-snapshot-i586-mingw32msvc-20140301-8956 3.4.0/rc1/ #8956 (Mar 1 2014)(MINGW32)
tested also with sdcc-3.3.0

Attached files with c sample code and resulting asm file.

2 Attachments

Discussion

  • Visenri

    Visenri - 2014-03-04

    Browsing sdcc pic bug repports i found that this is a special case of a bug reported for pic16 of this bug, as i suspected, this bug is present in all pic variants:

    https://sourceforge.net/p/sdcc/bugs/2151/

    It also points the possible problem with interrupt code.
    It's been a year since it has been reported, can it be fixed easily at least for this simple case (simple bit clear)?.

     
  • Visenri

    Visenri - 2014-03-04

    adding a cast fixed the problem, but if var is bigger than a char this can cause errors.

    #define bit_clear(var,bitno) ((var) &= (unsigned char)~(1 << (bitno)))
    
        BSF _PORTCbits,0
    ;   .line   12; "newmain.c" PORTCbits.RC0 = 0;
        BCF _PORTCbits,0
    ;   .line   14; "newmain.c" bit_set(PORTC,0);
        BSF _PORTC,0
    ;   .line   15; "newmain.c" bit_clear(PORTC,0);
        BCF _PORTC,0
    

    Sorry for not reading formatting help before posting the bug.

     
  • Maarten Brock

    Maarten Brock - 2014-03-05
    • Description has changed:

    Diff:

    --- old
    +++ new
    @@ -4,20 +4,27 @@
         PORTCbits.RC0 = 0;
    
     compiles to:
    +
    +~~~~
     ;  .line   11; "newmain.c" PORTCbits.RC0 = 1;
        BSF _PORTCbits,0
     ;  .line   12; "newmain.c" PORTCbits.RC0 = 0;
        BCF _PORTCbits,0
    +~~~~
    
     Setting bits in a portable way does not generate the expected result:
    
    +~~~~
     #define bit_set(var,bitno)   ((var) |=   1 << (bitno) )
     #define bit_clear(var,bitno) ((var) &= ~(1 << (bitno)))
    
         bit_set(PORTC,0);
         bit_clear(PORTC,0);
    +~~~~
    
     compiles to:
    +
    +~~~~
     ;  .line   14; "newmain.c" bit_set(PORTC,0);
        BSF _PORTC,0
     ;  .line   15; "newmain.c" bit_clear(PORTC,0);
    @@ -28,6 +35,7 @@
        ANDWF   r0x1000,W
        BANKSEL _PORTC
        MOVWF   _PORTC
    +~~~~
    
     As you can see, set bit operation is recognized correctly, but clear bit operation results in a long sequence anding the port with a temporal register and writing it back to the port.
     This is far from optimal code and can cause problems if the same port is being used in the interrupt code.
    
     
  • Philipp Klaus Krause

    • Priority: 5 --> 1
     
  • Philipp Klaus Krause

    Reducing priority, since the pic14 port is still experimental and has far bigger issues than some missing optimization.

    Philipp

     

Log in to post a comment.