Problem with init of structure bit fields

Problem with init of structure bit fields

[Stelian Pop]

Hi,

I'm doing a port of gcc 4.3.3 on a custom architecture and I'm having trouble
when initializing the bit fields of  a structure.

The testcase is based on a modified gcc torture testcase, the natural
registers are 16 bits, and long long is defined to be 64 bit wide:

struct itmp
{
  long long int pad :   30; 
  long long int field : 34; 
};

struct itmp itmp = {0x123LL, 0x123456LL};

int main(void)
{
  itmp.field = 0x42;
  return 1;
}

Running the above example gives (compiled with -O0...):

12	  itmp.field = 0x42;
(gdb) p/x itmp
$1 = {pad = 0x123, field = 0x123456}
(gdb) n
13	  return 1;
(gdb) p/x itmp
$2 = {pad = 0x123, field = 0x0}

If I use 32 bits for pad and 32 bits for field, the result is correct.
Also, if I use 'long' instead of 'long long' (and change the bit lengths
of course), it works too.

Looking at the RTL shows the problem right from the beginning, in the
expand pass (there is no reference to the constant 66 = 0x42 in the RTL
below):

;; itmp.field = 66
(insn 5 4 6 991118-1.c:12 (set (reg/f:HI 25) 
        (symbol_ref:HI ("itmp") [flags 0x2] <var_decl 0xb7c460b0 itmp>)) -1
(nil))

(insn 6 5 7 991118-1.c:12 (set (reg:HI 26) 
        (reg/f:HI 25)) -1 (nil))

(insn 7 6 8 991118-1.c:12 (set (reg/f:HI 27) 
        (plus:HI (reg/f:HI 25) 
            (const_int 6 [0x6]))) -1 (nil))

(insn 8 7 9 991118-1.c:12 (set (reg:HI 28) 
        (const_int 0 [0x0])) -1 (nil))

(insn 9 8 10 991118-1.c:12 (set (mem/s/j/c:HI (reg/f:HI 27) [0+6 S2 A16])
        (reg:HI 28)) -1 (nil))

(insn 10 9 11 991118-1.c:12 (set (reg:HI 29)
        (reg/f:HI 25)) -1 (nil))

(insn 11 10 12 991118-1.c:12 (set (reg/f:HI 30)
        (plus:HI (reg/f:HI 25)
            (const_int 4 [0x4]))) -1 (nil))

(insn 12 11 13 991118-1.c:12 (set (reg:HI 31)
        (const_int 0 [0x0])) -1 (nil))

(insn 13 12 14 991118-1.c:12 (set (mem/s/j/c:HI (reg/f:HI 30) [0+4 S2 A16])
        (reg:HI 31)) -1 (nil))

(insn 14 13 15 991118-1.c:12 (set (reg:HI 32)
        (reg/f:HI 25)) -1 (nil))

(insn 15 14 16 991118-1.c:12 (set (reg/f:HI 33)
        (plus:HI (reg/f:HI 25)
            (const_int 2 [0x2]))) -1 (nil))

(insn 16 15 17 991118-1.c:12 (set (reg:HI 34)
        (mem/s/j/c:HI (reg/f:HI 33) [0+2 S2 A16])) -1 (nil))

(insn 17 16 18 991118-1.c:12 (set (reg:HI 36)
        (const_int -4 [0xfffffffc])) -1 (nil))

(insn 18 17 19 991118-1.c:12 (set (reg:HI 35)
        (and:HI (reg:HI 34)
            (reg:HI 36))) -1 (nil))

(insn 19 18 0 991118-1.c:12 (set (mem/s/j/c:HI (reg/f:HI 33) [0+2 S2 A16])
        (reg:HI 35)) -1 (nil))


Any idea on what is happenning here ? Am I missing some standard
patterns in my .md file ?

Thanks !

-- 
Stelian Pop <stelian@popies.net>

Re: Problem with init of structure bit fields

[Stelian Pop]

On Wed, Jun 03, 2009 at 07:49:29PM +0200, Stelian Pop wrote:

> I'm doing a port of gcc 4.3.3 on a custom architecture and I'm having trouble
> when initializing the bit fields of  a structure.

Ok, after further analysis, it looks like a genuine bug in gcc,
in store_bit_field_1(): when a field is bigger than a word, the
rvalue is splitted in several words, after being placed in a
smallest_mode_for_size() operand.

But the logic for adressing the words of this operand is buggy
in the WORDS_BIG_ENDIAN case, the most signficant words being
used instead of the least significant ones.

The following patch corrects this, and makes the
gcc.c-torture/execute/991118-1.c testcase work correctly on my platform.

Stelian.

diff --git a/gcc/expmed.c b/gcc/expmed.c
index dc61de7..03c60a8 100644
--- a/gcc/expmed.c
+++ b/gcc/expmed.c
@@ -582,7 +582,10 @@ store_bit_field_1 (rtx str_rtx, unsigned HOST_WIDE_INT bitsize,
 	{
 	  /* If I is 0, use the low-order word in both field and target;
 	     if I is 1, use the next to lowest word; and so on.  */
-	  unsigned int wordnum = (backwards ? nwords - i - 1 : i);
+	  unsigned int wordnum = (backwards
+				  ? GET_MODE_SIZE(fieldmode) / UNITS_PER_WORD
+				    - i - 1
+				  : i);
 	  unsigned int bit_offset = (backwards
 				     ? MAX ((int) bitsize - ((int) i + 1)
 					    * BITS_PER_WORD,
-- 
Stelian Pop <stelian@popies.net>

Re: Problem with init of structure bit fields

[Ian Lance Taylor]

Stelian Pop <stelian@popies.net> writes:

> On Wed, Jun 03, 2009 at 07:49:29PM +0200, Stelian Pop wrote:
>
>> I'm doing a port of gcc 4.3.3 on a custom architecture and I'm having trouble
>> when initializing the bit fields of  a structure.
>
> Ok, after further analysis, it looks like a genuine bug in gcc,
> in store_bit_field_1(): when a field is bigger than a word, the
> rvalue is splitted in several words, after being placed in a
> smallest_mode_for_size() operand.
>
> But the logic for adressing the words of this operand is buggy
> in the WORDS_BIG_ENDIAN case, the most signficant words being
> used instead of the least significant ones.
>
> The following patch corrects this, and makes the
> gcc.c-torture/execute/991118-1.c testcase work correctly on my platform.
>
> Stelian.
>
> diff --git a/gcc/expmed.c b/gcc/expmed.c
> index dc61de7..03c60a8 100644
> --- a/gcc/expmed.c
> +++ b/gcc/expmed.c
> @@ -582,7 +582,10 @@ store_bit_field_1 (rtx str_rtx, unsigned HOST_WIDE_INT bitsize,
>  	{
>  	  /* If I is 0, use the low-order word in both field and target;
>  	     if I is 1, use the next to lowest word; and so on.  */
> -	  unsigned int wordnum = (backwards ? nwords - i - 1 : i);
> +	  unsigned int wordnum = (backwards
> +				  ? GET_MODE_SIZE(fieldmode) / UNITS_PER_WORD
> +				    - i - 1
> +				  : i);
>  	  unsigned int bit_offset = (backwards
>  				     ? MAX ((int) bitsize - ((int) i + 1)
>  					    * BITS_PER_WORD,

Your patch looks correct.  However, it makes me wonder how the test case
passes on existing big-endian platforms, such as the PowerPC.  Can you
explain how that works?

Ian

Re: Problem with init of structure bit fields

[Stelian Pop]

On Wed, Jun 03, 2009 at 01:07:08PM -0700, Ian Lance Taylor wrote:

> > -	  unsigned int wordnum = (backwards ? nwords - i - 1 : i);
> > +	  unsigned int wordnum = (backwards
> > +				  ? GET_MODE_SIZE(fieldmode) / UNITS_PER_WORD
> > +				    - i - 1
> > +				  : i);
> >  	  unsigned int bit_offset = (backwards
> >  				     ? MAX ((int) bitsize - ((int) i + 1)
> >  					    * BITS_PER_WORD,
> 
> Your patch looks correct.  However, it makes me wonder how the test case
> passes on existing big-endian platforms, such as the PowerPC.  Can you
> explain how that works?

I'm not sure, but it is related to the smaller word size on my platform
and the size of the field (a 64 bit long long).

The 991118-1.c test case passes ok on my platform when the word
size is 32 or 16 bits (my processor has configurable word sizes). The only
configuration that fails is when the word size is 8 bits...

When I tried to simplify the testcase I obtained the simple assignment
problem I posted in the original mail. And that problem was visible in
both 8 and 16 bits configurations.

So I suspect the problem won't be seen on PowerPC unless someone does
TI or OI bit fields...

-- 
Stelian Pop <stelian@popies.net>