[Bug target/67644] New: [SH] double load on volatile bitfield mem

[Bug target/67644] New: [SH] double load on volatile bitfield mem

[olegendo at gcc dot gnu.org]

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

            Bug ID: 67644
           Summary: [SH] double load on volatile bitfield mem
           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*-*-*

The following:

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_1 (volatile USRSTR* x)
{
  x->ICR0.BIT.BIT5 |= 1;
}

compiled with -O2 -m4 -ml results in:
        mov.b   @r4,r1
        mov.b   @r4,r0
        or      #4,r0
        mov.b   r0,@r4
        rts
        nop

The double load looks wrong.  With normal memory it might do no harm, but when
accessing hardware registers this might result in wrong behavior.  For
instance, sometimes hardware register reads clear status bits etc.

It seems this happens only when the bitfield is read and written back.  If it's
only read, there is only one load:

int test_2 (volatile USRSTR* x)
{
  return x->ICR0.BIT.BIT5;
}

        mov.b   @r4,r0
        tst     #4,r0
        mov     #-1,r0
        rts
        negc    r0,r0

And it happens only when there are bitfields involved:

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

        mov.b   @r4,r0
        extu.b  r0,r0
        or      #4,r0
        mov.b   r0,@r4
        rts     
        nop

The problem is present on trunk and GCC 5.

[Bug target/67644] [SH] double load on volatile bitfield mem

[olegendo at gcc dot gnu.org]

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

--- Comment #1 from Oleg Endo <olegendo at gcc dot gnu.org> ---
I've just checked with the current GCC 4.9 branch.  It happens there, too.

[Bug target/67644] [SH] double load on volatile bitfield mem

[olegendo at gcc dot gnu.org]

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

Oleg Endo <olegendo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|sh*-*-*                     |sh*-*-* rx*-*-*
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-09-20
     Ever confirmed|0                           |1

--- Comment #2 from Oleg Endo <olegendo at gcc dot gnu.org> ---
The same happens on GCC 5 rx-elf:

        mov.B   [r1], r14
        mov.B   [r1], r14
        bset    #2, r14
        mov.B   r14, [r1]
        rts

[Bug rtl-optimization/67644] double load on volatile bitfield mem

[olegendo at gcc dot gnu.org]

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

--- Comment #3 from Oleg Endo <olegendo at gcc dot gnu.org> ---
It somehow makes sense ...

  x->ICR0.BIT.BIT5 |= 1;

or maybe better

  x->ICR0.BIT.BIT5 ^= 1;

is a bitfield read and a bitfield write.
A bitfield write implies a bitfield read-modify-write, and thus we get two
reads.

It is sort of documented (at least the docs advice not to use volatile
bitfields for hardware access etc).  But it's still counter intuitive and a
silly trap, because it looks very similar to normal volatile variables.

Anyway, I guess this is an invalid PR...

[Bug rtl-optimization/67644] double load on volatile bitfield mem

[olegendo at gcc dot gnu.org]

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

--- Comment #4 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Hm, maybe it'd be good to add a warning (enabled by default, can be disabled)
if volatile bitfields are used.  To me it looks like volatile bitfields have
almost no use (the way they are implemented by GCC now) and probably everybody
is just trying to avoid them anyway.

[Bug rtl-optimization/67644] double load on volatile bitfield mem

[olegendo at gcc dot gnu.org]

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

--- Comment #5 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Possibly related: PR 50521, PR 56997