[Bug target/67645] New: [SH] Inefficient single bit bitfield operations

[Bug target/67645] New: [SH] Inefficient single bit bitfield operations

[olegendo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67645

            Bug ID: 67645
           Summary: [SH] Inefficient single bit bitfield operations
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: olegendo at gcc dot gnu.org
  Target Milestone: ---
            Target: sh*-*-*

Ignoring the volatile bitfield double load issue of PR 67644, there seems to be
some room for improvements w.r.t. operations on single bit bitfields.  The
examples below were compiled -m4 -ml -O2.

struct USRSTR
{
  union
  {
    unsigned char BYTE;
    struct
    {
      unsigned char BIT7:1;
      unsigned char BIT6:1;
      unsigned char BIT5:1;
      unsigned char BIT4:1;
      unsigned char BIT3:1;
      unsigned char BIT2:1;
      unsigned char BIT1:1;
      unsigned char BIT0:1;
    }
    BIT;
  }
  ICR0;
};

void test_0 (volatile USRSTR* x)
{
  x->ICR0.BIT.BIT5 |= 0x10;
}

        mov.b   @r4,r0
        mov.b   @r4,r2
        mov     #-5,r1      0b...1111'1011
        and     #4,r0       isolate bit #3 from mem
        and     r2,r1       clear bit #3 from mem
        or      r1,r0       effectively a nop
        mov.b   r0,@r4
        rts
        nop

the equivalent without bitfields:

void test_0_1 (volatile unsigned char* x)
{
  x[0] |= 0;
}

        mov.b   @r4,r1
        mov.b   r1,@r4
        rts     
        nop


void test_2 (volatile USRSTR* x, unsigned char y)
{
  x->ICR0.BIT.BIT5 |= y & 1;
}

        mov.b   @r4,r0
        mov     #1,r2
        mov.b   @r4,r3
        tst     #4,r0
        mov     #-1,r1
        and     r2,r5
        negc    r1,r1
        or      r5,r1
        mov     r3,r5
        mov     #-5,r2
        and     r2,r5
        shll2   r1
        or      r5,r1
        mov.b   r1,@r4
        rts
        nop

the equivalent without bitfields:

void test2_1 (volatile unsigned char* x, unsigned char y)
{
  x[0] |= (y & 1) << 2;
}

        mov.b   @r4,r1
        mov     r5,r0
        shll2   r0
        and     #4,r0
        or      r0,r1
        mov.b   r1,@r4
        rts
        nop

In some cases, a shift sequence as above is a good choice, especially when the
bit positions can be reached with a 1,2,8,16 shift.  Although this particular
case can be slightly improved (reduce R0 reg pressure):

        mov.b   @r4,r1
        shlr    r5        use T bit to do the (x & 1)
        movt    r5
        shll2   r5
        or      r5,r1
        mov.b   r1,@r4
        rts
        nop


In some other cases, it might be better to not use shifts:

void test2_2 (volatile unsigned char* x, unsigned char y)
{
  x[0] |= ((y >> 7) & 1) << 3;
}

currently results in:
        mov     r5,r0
        mov     #-1,r2
        tst     #128,r0
        negc    r2,r1
        mov.b   @r4,r3
        shll2   r1
        add     r1,r1
        or      r3,r1
        mov.b   r1,@r4
        rts
        nop

which could be done as:
        mov     r5,r0
        tst     #128,r0    sign extract bit through T bit...
        subc    r0,r0
        not     r0,r0      ... into r0
        and     #8,r0      punch out one bit at constant position
        mov.b   @r4,r3
        or      r3,r0
        mov.b   r0,@r4
        rts
        nop

Interestingly, when using 'unsigned int' instead of 'unsigned char', the code
tends to be better:

void test2_7 (volatile unsigned int* x, unsigned int y)
{
  x[0] |= ((y >> 7) & 1) << 3;
}

        mov     r5,r0
        shlr2   r0
        mov.l   @r4,r1
        shlr2   r0
        and     #8,r0
        or      r0,r1
        mov.l   r1,@r4
        rts     
        nop