On Tue, Mar 10, 2009 at 2:44 PM, Hariharan Sandanagobalane
<harihar...@picochip.com> wrote:
> Hi,
> Since r144598, pr39339.c has been failing on picochip. On investigation, it
> looks to me that the testcase is illegal.
>
> Relevant source code:
> struct C
> {
> unsigned int c;
> struct D
> {
> unsigned int columns : 4;
> unsigned int fore : 9;
> unsigned int back : 9;
> unsigned int fragment : 1;
> unsigned int standout : 1;
> unsigned int underline : 1;
> unsigned int strikethrough : 1;
> unsigned int reverse : 1;
> unsigned int blink : 1;
> unsigned int half : 1;
> unsigned int bold : 1;
> unsigned int invisible : 1;
> unsigned int pad : 1;
> } attr;
> };
>
> struct A
> {
> struct C *data;
> unsigned int len;
> };
>
> struct B
> {
> struct A *cells;
> unsigned char soft_wrapped : 1;
> };
>
> struct E
> {
> long row, col;
> struct C defaults;
> };
>
> __attribute__ ((noinline))
> void foo (struct E *screen, unsigned int c, int columns, struct B *row)
> {
> struct D attr;
> long col;
> int i;
> col = screen->col;
> attr = screen->defaults.attr;
> attr.columns = columns;
> row->cells->data[col].c = c;
> row->cells->data[col].attr = attr;
> col++;
> attr.fragment = 1;
> for (i = 1; i < columns; i++)
> {
> row->cells->data[col].c = c;
> row->cells->data[col].attr = attr;
> col++;
> }
> }
>
> int
> main (void)
> {
> struct E e = {.row = 5,.col = 0,.defaults =
> {6, {-1, -1, -1, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0}} };
> struct C c[4];
> struct A a = { c, 4 };
> struct B b = { &a, 1 };
> struct D d;
> __builtin_memset (&c, 0, sizeof c);
> foo (&e, 65, 2, &b);
> d = e.defaults.attr;
> d.columns = 2;
> if (__builtin_memcmp (&d, &c[0].attr, sizeof d))
> __builtin_abort ();
> d.fragment = 1;
> if (__builtin_memcmp (&d, &c[1].attr, sizeof d))
> __builtin_abort ();
> return 0;
> }
>
>
> In picochip, PCC_BITFIELD_TYPE_MATTERS is set and int is 16-bits, so the
> structure D becomes 6 bytes, with 3-bit padding between fore and back.
>
> At SRA the code becomes
>
> ;; Function foo (foo)
>
> foo (struct E * screen, unsigned int c, int columns, struct B * row)
> {
> unsigned int attr$B32F16;
> <unnamed-unsigned:6> attr$B26F6;
> <unnamed-unsigned:9> attr$back;
> <unnamed-unsigned:9> attr$fore;
> <unnamed-unsigned:1> attr$fragment;
> int i;
> long int col;
> struct C * D.1267;
> unsigned int D.1266;
> unsigned int D.1265;
> struct C * D.1264;
> struct A * D.1263;
> <unnamed-unsigned:4> D.1262;
> unsigned char D.1261;
>
> <bb 2>:
> col_4 = screen_3(D)->col;
> attr$B32F16_36 = BIT_FIELD_REF <screen_3(D)->defaults.attr, 16, 32>;
> attr$B26F6_37 = BIT_FIELD_REF <screen_3(D)->defaults.attr, 6, 26>;
> attr$back_38 = screen_3(D)->defaults.attr.back;
> attr$fore_39 = screen_3(D)->defaults.attr.fore;
> attr$fragment_40 = screen_3(D)->defaults.attr.fragment;
> D.1261_6 = (unsigned char) columns_5(D);
> D.1262_7 = (<unnamed-unsigned:4>) D.1261_6;
> D.1263_9 = row_8(D)->cells;
> D.1264_10 = D.1263_9->data;
> D.1265_11 = (unsigned int) col_4;
> D.1266_12 = D.1265_11 * 8;
> D.1267_13 = D.1264_10 + D.1266_12;
> D.1267_13->c = c_14(D);
> BIT_FIELD_REF <D.1267_13->attr, 16, 32> = attr$B32F16_36;
> BIT_FIELD_REF <D.1267_13->attr, 6, 26> = attr$B26F6_37;
> D.1267_13->attr.back = attr$back_38;
> D.1267_13->attr.fore = attr$fore_39;
> D.1267_13->attr.fragment = attr$fragment_40;
> D.1267_13->attr.columns = D.1262_7;
> col_20 = col_4 + 1;
> if (columns_5(D) > 1)
> goto <bb 3>;
> else
> goto <bb 4>;
>
> <bb 3>:
> # col_29 = PHI <col_32(3), col_20(2)>
> # i_30 = PHI <i_33(3), 1(2)>
> D.1265_24 = (unsigned int) col_29;
> D.1266_25 = D.1265_24 * 8;
> D.1267_26 = D.1264_10 + D.1266_25;
> D.1267_26->c = c_14(D);
> BIT_FIELD_REF <D.1267_26->attr, 16, 32> = attr$B32F16_36;
> BIT_FIELD_REF <D.1267_26->attr, 6, 26> = attr$B26F6_37;
> D.1267_26->attr.back = attr$back_38;
> D.1267_26->attr.fore = attr$fore_39;
> D.1267_26->attr.fragment = 1;
> D.1267_26->attr.columns = D.1262_7;
> col_32 = col_29 + 1;
> i_33 = i_30 + 1;
> if (columns_5(D) > i_33)
> goto <bb 3>;
> else
> goto <bb 4>;
>
> <bb 4>:
> return;
>
> }
>
>
>
> ;; Function main (main)
>
> main ()
> {
> struct D d;
> struct B b;
> struct A a;
> struct C c[4];
> struct E e;
> int D.1279;
> int D.1276;
>
> <bb 2>:
> e.row = 5;
> e.col = 0;
> e.defaults.c = 6;
> e.defaults.attr.columns = 15;
> e.defaults.attr.fore = 511;
> e.defaults.attr.back = 511;
> e.defaults.attr.fragment = 1;
> e.defaults.attr.standout = 0;
> e.defaults.attr.underline = 1;
> e.defaults.attr.strikethrough = 0;
> e.defaults.attr.reverse = 1;
> e.defaults.attr.blink = 0;
> e.defaults.attr.half = 1;
> e.defaults.attr.bold = 0;
> e.defaults.attr.invisible = 1;
> e.defaults.attr.pad = 0;
> a.data = &c;
> a.len = 4;
> b.cells = &a;
> b.soft_wrapped = 1;
> __builtin_memset (&c, 0, 32);
> foo (&e, 65, 2, &b);
> d = e.defaults.attr;
> d.columns = 2;
> D.1276_1 = __builtin_memcmp (&d, &c[0].attr, 6);
> if (D.1276_1 != 0)
> goto <bb 3>;
> else
> goto <bb 4>;
>
> <bb 3>:
> __builtin_abort ();
>
> <bb 4>:
> d.fragment = 1;
> D.1279_2 = __builtin_memcmp (&d, &c[1].attr, 6);
> if (D.1279_2 != 0)
> goto <bb 5>;
> else
> goto <bb 6>;
>
> <bb 5>:
> __builtin_abort ();
>
> <bb 6>:
> return 0;
>
> }
>
>
> Note that padding bits (13,16) are not copied over in bb_2 in function foo.
> main then does a memcmp, which fails because the padding bits are different.
>
> From C99 standards (p328), 265) The contents of ‘‘holes’’ used as padding
> for purposes of alignment within structure objects are
> indeterminate. Strings shorter than their allocated space and unions may
> also cause problems in comparison.
>
> Is this just that the testcase is illegal on 16-bit targets or is it SRA's
> responsibility to copy the padding bits?
The memcmp assumes the bitfield is packed. Does the testcase work
for you if you make the structure so?
Richard.
> TIA,
> Regards
> Hari
>
