Bug 45637 - Suspicious code for bit fields
Summary: Suspicious code for bit fields
Status: RESOLVED INVALID
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.5.1
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-10 15:10 UTC by Sebastian Huber
Modified: 2010-09-10 15:59 UTC (History)
1 user (show)

See Also:
Host: x86_64-unknown-linux-gnu
Target: powerpc-rtems4.11
Build: x86_64-unknown-linux-gnu
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sebastian Huber 2010-09-10 15:10:31 UTC
Please have a look at this test case:

#include <stdint.h>

struct type1 {
        union {
                uint8_t reg;
                struct {
                        uint8_t : 7;
                        uint8_t b : 1;
                } bit;
        
        } foo [1];
        uint8_t bar;
};

volatile struct type1 *var1;

void f1(int i)
{
	var1->foo [i].bit.b = 1;
	var1->foo [0].bit.b = 1;
}

struct type2 {
        union {
                uint8_t reg;
                struct {
                        uint8_t : 7;
                        uint8_t b : 1;
                } bit;
        
        } foo [1];
        uint16_t bar;
};

volatile struct type2 *var2;

void f2(int i)
{
	var2->foo [i].bit.b = 1;
	var2->foo [0].bit.b = 1;
}

struct type3 {
        union {
                uint8_t reg;
                struct {
                        uint8_t : 7;
                        uint8_t b : 1;
                } bit;
        
        } foo [1];
        uint32_t bar;
};

volatile struct type3 *var3;

void f3(int i)
{
	var3->foo [i].bit.b = 1;
	var3->foo [0].bit.b = 1;
}

And the produced assembler file generated with (powerpc-rtems4.11-gcc -O2 -c -mcpu=8540 -meabi -msdata -fno-common -save-temps):

	.file	"bitfields.c"
	.gnu_attribute 4, 2
	.gnu_attribute 8, 1
	.gnu_attribute 12, 1
	.section	".text"
	.align 2
	.globl f1
	.type	f1, @function
f1:
	lwz 9,var1@sda21(0)
	lbzx 0,9,3
	ori 0,0,1
	stbx 0,9,3
	lbz 0,0(9)
	ori 0,0,1
	stb 0,0(9)
	blr
	.size	f1, .-f1
	.align 2
	.globl f2
	.type	f2, @function
f2:
	lwz 9,var2@sda21(0)
	lbzx 0,9,3
	ori 0,0,1
	stbx 0,9,3
	lhz 0,0(9)
	ori 0,0,256
	sth 0,0(9)
	blr
	.size	f2, .-f2
	.align 2
	.globl f3
	.type	f3, @function
f3:
	lwz 9,var3@sda21(0)
	li 11,1
	lbzx 0,9,3
	ori 0,0,1
	stbx 0,9,3
	lwz 0,0(9)
	rlwimi 0,11,24,7,7
	stw 0,0(9)
	blr
	.size	f3, .-f3
	.globl var1
	.globl var2
	.globl var3
	.section	.sbss,"aw",@nobits
	.align 2
	.type	var1, @object
	.size	var1, 4
var1:
	.zero	4
	.type	var2, @object
	.size	var2, 4
var2:
	.zero	4
	.type	var3, @object
	.size	var3, 4
var3:
	.zero	4
	.ident	"GCC: (GNU) 4.5.1 20100731 (RTEMS gcc-4.5.1-5.suse11.2/newlib-1.18.0-20.suse11.2)"

Here you can see, that the access to the 'foo' field depends on

1. index is constant or variable, and
2. the 'bar' field type.

I think that both dependencies are wrong.  In any case byte accesses should be used.
Comment 1 Andrew Pinski 2010-09-10 15:30:21 UTC
>1. index is constant or variable, and

Yes that is correct.  

>2. the 'bar' field type.

The alignment of the access is different in those cases.  

>In any case byte accesses should be used.
Why, word access is just as fast (if not faster) than a byte access on PPC.
Comment 2 Sebastian Huber 2010-09-10 15:43:28 UTC
(In reply to comment #1)
> >1. index is constant or variable, and
> 
> Yes that is correct.  
> 
> >2. the 'bar' field type.
> 
> The alignment of the access is different in those cases.

Sorry, the test case was not good.  If you expand foo [1] to foo [4] you still have this behavior.
  
> 
> >In any case byte accesses should be used.
> Why, word access is just as fast (if not faster) than a byte access on PPC.
> 

Yes, but we have 'volatile struct type1 *varN;'.  For volatile fields we should use accesses of the appropriate width.

The background is that a major hardware manufacturer provided structure definitions like the above test case for register definitions.
Comment 3 Andrew Pinski 2010-09-10 15:46:57 UTC
>For volatile fields we should use accesses of the appropriate width.

The PowerPC ABI has specific rules for accessing volatile bitfields and IIRC it says they should be doing the largest available (up to the register size) size.

This is different from the ARM ABI which says the opposite.
Comment 4 Sebastian Huber 2010-09-10 15:59:18 UTC
(In reply to comment #3)
> >For volatile fields we should use accesses of the appropriate width.
> 
> The PowerPC ABI has specific rules for accessing volatile bitfields and IIRC it
> says they should be doing the largest available (up to the register size) size.
> 
> This is different from the ARM ABI which says the opposite.
> 

Thank you very much for your comments.  I will investigate the PowerPC ABI issue and contact the manufacturer.  Ironically they produced the ABI and these register definitions.