#3070 Bug in PDK14/15 backend when accessing bitfields.

[Tim B.]

I have been trying again to use bitfields in SDCC to ease port access. There is still a bug, where SDCC generates incorrect code. to set or clear individual bits.

Example code (Blinky):

struct PORT_bits
{
    uint8_t p0:1;
    uint8_t p1:1;
    uint8_t p2:1;
    uint8_t p3:1;
    uint8_t p4:1;
    uint8_t p5:1;
    uint8_t p6:1;
    uint8_t p7:1;
}; // __attribute__((__packed__));

#define BF_PA   (*(volatile struct PORT_bits *)&PA)

void main(void)
{
    PAC |= _BV(LEDPIN);     // This syntax will infer set0/set1

    for (;;) {
        BF_PA.p0=1;
        delay_ms(500);

        BF_PA.p0=0;
        delay_ms(500);
    }
}

SDCC version:

SDCC : gbz80/tlcs90/z80n/pic14/ds400/pdk13/pdk14/pdk15 4.0.2 #11683 (Linux)

Here is what is emitted:
~~~
260 ; bitfields.c: 55: BF_PA.p0=1;
000040 01 2F 261 mov a, #0x01
a 000042 262 or _pa+0, a

...

  00004C FE 2F                  270     mov a, #0xfe

a 00004E 271 and _pa+0, a
~~~

AND io,a , OR io,a , unfortunately do not exist as opcodes. Therefore the assembler fails to assemble this. This needs to be replaced with set0 / set1.

[Tim B.]

The output again, something got mangled up:

                                    260 ;   bitfields.c: 55: BF_PA.p0=1;
      000040 01 2F                  261     mov a, #0x01
a     000042                        262     or  _pa+0, a

                                    269 ;   bitfields.c: 59: BF_PA.p0=0;
      00004C FE 2F                  270     mov a, #0xfe
a     00004E                        271     and _pa+0, a

[Philipp Klaus Krause]

Can you make the code sample compileable (it is currently missing _BV and PA)?

[Tim B.]

Of course!

I added some additional examples that work, but where inefficient code is generated. The bit transfer could make use of "swapc", while the bittest could use "t0sn".

__sfr __at(0x10)    pa;
#define PA          pa // __sfr __at(0x10) PA

struct PORT_bits
{
    uint8_t p0:1;
    uint8_t p1:1;
    uint8_t p2:1;
    uint8_t p3:1;
    uint8_t p4:1;
    uint8_t p5:1;
    uint8_t p6:1;
    uint8_t p7:1;
}; // __attribute__((__packed__));

#define BF_PA   (*(volatile struct PORT_bits *)&PA)

void main(void)
{
    for (;;) {
        BF_PA.p0=1;   // breaks

        BF_PA.p0=0;   // breaks

        if (BF_PA.p2) {  // works, but inefficient
            BF_PA.p0=1;  // breaks
            }

        BF_PA.p1=BF_PA.p3;  // works, but inefficient
    }
}

[Philipp Klaus Krause]

Thanks. I haven't seen code taking the address of an __sfr before, but there clearly is a bug, as SDCC should either generate correct code, or give an error.

[Tim B.]

Well, you know, this would be very useful to simplify direct access to individiual I/O pins. I believe the sdcc 8051 library also makes use of this?

Instead of "PA & =^BV(1);" ; you could write "PA.p1=0;" or even "PA1=0;"

[Philipp Klaus Krause]

But we definitely don't want generic pointers to be able to point to an __sfr:
Then we'd need a support function call for pointer writes (like we already need for pointer reads), which is far less efficient than idxm.
Anyway, I asked about this on the sdcc-devel mailing list to see what other SDCC developers think.

[Tim B.]

It's already used exactly like that in the HC08 library. See "\SDCC\include\hc08\".

struct __hc08_bits
{
  unsigned int bit0:1;
  unsigned int bit1:1;
  unsigned int bit2:1;
  unsigned int bit3:1;
  unsigned int bit4:1;
  unsigned int bit5:1;
  unsigned int bit6:1;
  unsigned int bit7:1;
};


_VOLDATA _UINT8 __at 0x00 PTA;     /* Port A Data Register */
  #define PTA0 ((struct __hc08_bits *)(&PTA))->bit0
  #define PTA1 ((struct __hc08_bits *)(&PTA))->bit1
  #define PTA2 ((struct __hc08_bits *)(&PTA))->bit2
  #define PTA3 ((struct __hc08_bits *)(&PTA))->bit3
  #define PTA4 ((struct __hc08_bits *)(&PTA))->bit4
  #define PTA5 ((struct __hc08_bits *)(&PTA))->bit5
  #define AWUL ((struct __hc08_bits *)(&PTA))->bit6 

[Philipp Klaus Krause]

hc08 (like s08 and stm8) does not have a separate __sfr address space. There, I/O is just volatile variables in memory (that happen to be a fixed addresses, but are accessed using the same instructions as data memory).

[Tim B.]

ok, I see. There seems to be a conflict in maintaining an orthogonal compiler with a non-orthogonal architecture :)

[Tim B.]

I have to add that AVR-GCC supports bitfield access to the I/O registers in exactly this way, also based on the SFR defines. The compiler is optimizing to use bit-set instructions that are only valid in the I/O space.

[Philipp Klaus Krause]

There is currently some discussion on the sdcc-devel list about this feature.
For z80, one could indeed allow pointers to the __sfr address space. But for pdk we have an extra problem: The pdk don't have indirect addressing on __sfr. So it would only work for cases, where compiler optimizations can deduct the exact address at compile-time.
Pointers to __sfr that sometimes work, but sometimes don't would also be confusing.

I wonder if it would be more helpful, if we could instead specify the type of an __sfr object, i.e. something like

__sfr struct PORT_BITS __at(0x10) BF_PA;

[Tim B.]

That surely would also be an option. But it would make it more difficult for people to assign their on bitfield naming when relying on standard includes.

[Maarten Brock]

No it doesn't. Everything that is placed with __at and has no initializer is overlapped meaning you can place as many variables there as you please.

[Philipp Klaus Krause]

Current SDCC gives an error when trying to use pointers to __sfr on ports where that is not supported. So at least, there no longer is a bug here.