> There is a well known bug in the ARM version of gcc 4.1.1

bugzilla bug# ?

A.K. wrote:
>> There is a well known bug in the ARM version of gcc 4.1.1
>
> bugzilla bug# ?

Bugzilla 23623 28568 Fixed with patch in CVS revision 112493

But I think the problem is more that just the use of volatile, I think
the compiler gets confused with the definition of the container for the
bitfield.

I.e.   unsigned long *  should produce operations on a 32 bit boundry,
but the compiler can try and be clever and add 1 to the address to make
it a byte boundary.   The ARMs internal registers to not support byte
alligned acceses.

In effect there are two problems.  I've no idea which ones the patch
fixes.

Michael Fischer wrote:
> Hello Jason,
>
> do you have a samll example which will fail?
> And do you have the fix?
>
> Best regards,
>
> Michael

I think the code below explains it.

#define __REG32 volatile unsigned long

typedef struct
{
  __REG32 Bit0 :1;
  __REG32 Bit1 :1;
  ...etc...
  __REG32 Backlight : 1;   //Bit10
  ...etc...
  __REG32 Bit31 :1;
}__Bitfield;

#define ADDRESS <address of my register>

#define ADDRESS 0xe0028000
#define REG (*( __REG32 *)(ADDRESS))
#define REG_bit (*(__Bitfield *)&REG)

When I access the register using the bitfield I get the code like the
following
Note the use of byte addressing and an offset of 1 byte and 3 bits

LCD_bit.Backlight=1;
     358:       e3a0320e        mov     r3, #-536870912 ; 0xe0000000
     35c:       e283390a        add     r3, r3, #163840 ; 0x28000
     360:       e5d32001        ldrb    r2, [r3, #1]
     364:       e3822004        orr     r2, r2, #4      ; 0x4
     368:       e5c32001        strb    r2, [r3, #1]

The workround is to do the following.

#define GPIO_BASE_ADDR    0xE0028000
#define IOSET0         (*(volatile unsigned long *)(GPIO_BASE_ADDR +
0x04))
#define BITPOS(x)      (1L<<x)
#define LCDLightOn()   { IO0SET=IO0SET|BITPOS(10); }

Which produces the follwing code:
  LCDLightOn();
      f4:  e3a0224e   mov  r2, #-536870908  ; 0xe0000004
      f8:  e282290a   add  r2, r2, #163840  ; 0x28000
      fc:  e3a0324e   mov  r3, #-536870908  ; 0xe0000004
     100:  e283390a   add  r3, r3, #163840  ; 0x28000
     104:  e5933000   ldr  r3, [r3]
     108:  e3833b01   orr  r3, r3, #1024  ; 0x400
     10c:  e5823000   str  r3, [r2]

Which is two extra instructions and much less readable code.

IMO there should be a #pragma or _attribute_ that says that bit
structure optimisation can't be used for this variable.  The
optimisation used above is redundant (unless it were an 8 bit processor,
which it is not) as 32bit access are just as costly in time as 8 bit.
I.e. long means 32 bit accesses as it should.

I've tried to lobby the gcc team to do this with no success.  Perhaps it
needs the backing of a large number of ARM users to get a resolution.

I got the following response:
----
No, this doesn't fit the critera for a regression, so it won't be fixed
in FSF releases.

However, I think the CodeSourcery distributions have this feature in a
4.1 build.
----

The compiler I'm using (4.1.0, -Os) emits this code for your IO0SET
example:
  ldr  r2, .L3
  ldr  r3, [r2, #4]
  orr  r3, r3, #1024
  str  r3, [r2, #4]

But the example is wrong anyway, because those I/O registers are
designed to make operations line
   IO0SET |= some_bit
unnecessary. Instead you should use
   IO0SET = some_bit
to set a bit, and
   IO0CLR = some_bit
to clear it. Besides creating shorter code, this kind of
bitmanipulations is interrupt-safe. Load-Op-Store however is not.

The bitfield style is not a reasonable representation for ARM
peripherals having separate set/clear registers (LPC2000 some, SAM7
plenty). If LCD_bit.Backlight=1 sets a bit using the IO0SET register,
LCD_bit.Backlight=0 does not clear it.

I agree the example is wrong, this was hacked out of code for another
purpose.  As for the LCD_bit example, this actually uses IOPIN.  The
reason for using IOPIN is that it makes the code portable when the
peripheral (not an LCD, but that is all I have on my dev board) later
moves to an FPGA.

The purpose of this thread is to discuss the bug in GCC, not my ability
to write accurate examples of workrounds.  Do you have any furthur
comments on this?


A.K. wrote:
> The compiler I'm using (4.1.0, -Os) emits this code for your IO0SET
> example:
>   ldr  r2, .L3
>   ldr  r3, [r2, #4]
>   orr  r3, r3, #1024
>   str  r3, [r2, #4]
>
> But the example is wrong anyway, because those I/O registers are
> designed to make operations line
>    IO0SET |= some_bit
> unnecessary. Instead you should use
>    IO0SET = some_bit
> to set a bit, and
>    IO0CLR = some_bit
> to clear it. Besides creating shorter code, this kind of
> bitmanipulations is interrupt-safe. Load-Op-Store however is not.
>
> The bitfield style is not a reasonable representation for ARM
> peripherals having separate set/clear registers (LPC2000 some, SAM7
> plenty). If LCD_bit.Backlight=1 sets a bit using the IO0SET register,
> LCD_bit.Backlight=0 does not clear it.