Message ID | 20120425114750.GV16117@tyan-ft48-01.lab.bos.redhat.com |
---|---|
State | New |
Headers | show |
On Wed, 25 Apr 2012, Jakub Jelinek wrote: > Hi! > > This patch fixes the attached execute/ testcases. In some cases > get_best_mode was ignoring bitregion_* restrictions and returned > mode that overlapped into adjacent non-bitfields, or store_bit_field_1 > for insv could ignore those restrictions completely. > After fixing that I've run into several issues that the other parts > of the patch fix, namely that get_best_mode returning VOIDmode more often > now may lead to store_split_bit_field being called when it wasn't before, > and when approaching the bitregion_end point we can't just keep using > large units (modes), but need to decrease their size in order not to > overwrite adjacent non-bitfields. The get_best_mode dropping of % align > change prevents a mutual recursion, e.g. for bitregion_end == 103 > it would prevent using > QImode even far before the bit region boundary, > even when not touching anything close to that bit region end. > But the extra > + && (bitregion_end == 0 > + || bitpos - (bitpos % unit) + unit <= bitregion_end + 1)) > needed to be added after that change, otherwise we'd regress on > c-c++-common/simulate-thread/bitfields-2.c. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Ok. Thanks, Richard. > 2012-04-25 Jakub Jelinek <jakub@redhat.com> > > PR middle-end/52979 > * stor-layout.c (get_best_mode): Don't return mode with bitsize > larger than maxbits. Don't compute maxbits modulo align. > Also check that unit bytes long store at bitpos / unit * unit > doesn't affect bits beyond bitregion_end. > * expmed.c (store_bit_field_1): Avoid trying insv if OP_MODE MEM > would not fit into bitregion_start ... bitregion_end + 1 bit > region. > (store_split_bit_field): Decrease unit close to end of bitregion_end > if access is restricted in order to avoid mutual recursion. > > * gcc.c-torture/compile/pr52979-1.c: New test. > * gcc.c-torture/execute/pr52979-1.c: New test. > * gcc.c-torture/execute/pr52979-2.c: New test. > > --- gcc/stor-layout.c.jj 2012-03-26 11:53:20.000000000 +0200 > +++ gcc/stor-layout.c 2012-04-25 10:26:18.881631697 +0200 > @@ -2624,7 +2624,7 @@ get_best_mode (int bitsize, int bitpos, > if (!bitregion_end) > maxbits = MAX_FIXED_MODE_SIZE; > else > - maxbits = (bitregion_end - bitregion_start) % align + 1; > + maxbits = bitregion_end - bitregion_start + 1; > > /* Find the narrowest integer mode that contains the bit field. */ > for (mode = GET_CLASS_NARROWEST_MODE (MODE_INT); mode != VOIDmode; > @@ -2645,7 +2645,10 @@ get_best_mode (int bitsize, int bitpos, > (Though at least one Unix compiler ignores this problem: > that on the Sequent 386 machine. */ > || MIN (unit, BIGGEST_ALIGNMENT) > align > - || (largest_mode != VOIDmode && unit > GET_MODE_BITSIZE (largest_mode))) > + || (largest_mode != VOIDmode && unit > GET_MODE_BITSIZE (largest_mode)) > + || unit > maxbits > + || (bitregion_end > + && bitpos - (bitpos % unit) + unit > bitregion_end + 1)) > return VOIDmode; > > if ((SLOW_BYTE_ACCESS && ! volatilep) > @@ -2663,7 +2666,9 @@ get_best_mode (int bitsize, int bitpos, > && unit <= MIN (align, BIGGEST_ALIGNMENT) > && unit <= maxbits > && (largest_mode == VOIDmode > - || unit <= GET_MODE_BITSIZE (largest_mode))) > + || unit <= GET_MODE_BITSIZE (largest_mode)) > + && (bitregion_end == 0 > + || bitpos - (bitpos % unit) + unit <= bitregion_end + 1)) > wide_mode = tmode; > } > > --- gcc/expmed.c.jj 2012-04-19 11:09:13.000000000 +0200 > +++ gcc/expmed.c 2012-04-24 17:14:34.264897874 +0200 > @@ -640,7 +640,13 @@ store_bit_field_1 (rtx str_rtx, unsigned > && !(MEM_P (op0) && MEM_VOLATILE_P (op0) > && flag_strict_volatile_bitfields > 0) > && ! ((REG_P (op0) || GET_CODE (op0) == SUBREG) > - && (bitsize + bitpos > GET_MODE_BITSIZE (op_mode)))) > + && (bitsize + bitpos > GET_MODE_BITSIZE (op_mode))) > + /* Do not use insv if the bit region is restricted and > + op_mode integer at offset doesn't fit into the > + restricted region. */ > + && !(MEM_P (op0) && bitregion_end > + && bitnum - bitpos + GET_MODE_BITSIZE (op_mode) > + > bitregion_end + 1)) > { > struct expand_operand ops[4]; > int xbitpos = bitpos; > @@ -760,7 +766,7 @@ store_bit_field_1 (rtx str_rtx, unsigned > || GET_MODE_BITSIZE (GET_MODE (op0)) > maxbits > || (op_mode != MAX_MACHINE_MODE > && GET_MODE_SIZE (GET_MODE (op0)) > GET_MODE_SIZE (op_mode))) > - bestmode = get_best_mode (bitsize, bitnum, > + bestmode = get_best_mode (bitsize, bitnum, > bitregion_start, bitregion_end, > MEM_ALIGN (op0), > (op_mode == MAX_MACHINE_MODE > @@ -1096,6 +1102,16 @@ store_split_bit_field (rtx op0, unsigned > offset = (bitpos + bitsdone) / unit; > thispos = (bitpos + bitsdone) % unit; > > + /* When region of bytes we can touch is restricted, decrease > + UNIT close to the end of the region as needed. */ > + if (bitregion_end > + && unit > BITS_PER_UNIT > + && bitpos + bitsdone - thispos + unit > bitregion_end + 1) > + { > + unit = unit / 2; > + continue; > + } > + > /* THISSIZE must not overrun a word boundary. Otherwise, > store_fixed_bit_field will call us again, and we will mutually > recurse forever. */ > --- gcc/testsuite/gcc.c-torture/compile/pr52979-1.c.jj 2012-04-25 08:27:48.188902802 +0200 > +++ gcc/testsuite/gcc.c-torture/compile/pr52979-1.c 2012-04-25 08:27:29.000000000 +0200 > @@ -0,0 +1,15 @@ > +/* PR middle-end/52979 */ > + > +struct S > +{ > + unsigned int a : 16, b : 16, c : 16, d : 16, e : 14; > + unsigned int f : 4, g : 14, h : 8; > + char i; > + int j; > +}; > + > +void > +foo (struct S *s) > +{ > + s->f = 1; > +} > --- gcc/testsuite/gcc.c-torture/execute/pr52979-1.c.jj 2012-04-24 17:20:51.246662745 +0200 > +++ gcc/testsuite/gcc.c-torture/execute/pr52979-1.c 2012-04-24 17:21:03.526586921 +0200 > @@ -0,0 +1,40 @@ > +/* PR middle-end/52979 */ > + > +extern void abort (void); > +int c, d, e; > + > +void > +foo (void) > +{ > +} > + > +struct __attribute__((packed)) S { int g : 31; int h : 6; }; > +struct S a = { 1 }; > +static struct S b = { 1 }; > + > +void > +bar (void) > +{ > + a.h = 1; > + struct S f = { }; > + b = f; > + e = 0; > + if (d) > + c = a.g; > +} > + > +void > +baz (void) > +{ > + bar (); > + a = b; > +} > + > +int > +main () > +{ > + baz (); > + if (a.g) > + abort (); > + return 0; > +} > --- gcc/testsuite/gcc.c-torture/execute/pr52979-2.c.jj 2012-04-24 17:20:54.603642467 +0200 > +++ gcc/testsuite/gcc.c-torture/execute/pr52979-2.c 2012-04-24 17:19:15.000000000 +0200 > @@ -0,0 +1,40 @@ > +/* PR middle-end/52979 */ > + > +extern void abort (void); > +int c, d, e; > + > +void > +foo (void) > +{ > +} > + > +struct __attribute__((packed)) S { int g : 31; int h : 6; }; > +static struct S b = { 1 }; > +struct S a = { 1 }; > + > +void > +bar (void) > +{ > + a.h = 1; > + struct S f = { }; > + b = f; > + e = 0; > + if (d) > + c = a.g; > +} > + > +void > +baz (void) > +{ > + bar (); > + a = b; > +} > + > +int > +main () > +{ > + baz (); > + if (a.g) > + abort (); > + return 0; > +} > > Jakub > >
--- gcc/stor-layout.c.jj 2012-03-26 11:53:20.000000000 +0200 +++ gcc/stor-layout.c 2012-04-25 10:26:18.881631697 +0200 @@ -2624,7 +2624,7 @@ get_best_mode (int bitsize, int bitpos, if (!bitregion_end) maxbits = MAX_FIXED_MODE_SIZE; else - maxbits = (bitregion_end - bitregion_start) % align + 1; + maxbits = bitregion_end - bitregion_start + 1; /* Find the narrowest integer mode that contains the bit field. */ for (mode = GET_CLASS_NARROWEST_MODE (MODE_INT); mode != VOIDmode; @@ -2645,7 +2645,10 @@ get_best_mode (int bitsize, int bitpos, (Though at least one Unix compiler ignores this problem: that on the Sequent 386 machine. */ || MIN (unit, BIGGEST_ALIGNMENT) > align - || (largest_mode != VOIDmode && unit > GET_MODE_BITSIZE (largest_mode))) + || (largest_mode != VOIDmode && unit > GET_MODE_BITSIZE (largest_mode)) + || unit > maxbits + || (bitregion_end + && bitpos - (bitpos % unit) + unit > bitregion_end + 1)) return VOIDmode; if ((SLOW_BYTE_ACCESS && ! volatilep) @@ -2663,7 +2666,9 @@ get_best_mode (int bitsize, int bitpos, && unit <= MIN (align, BIGGEST_ALIGNMENT) && unit <= maxbits && (largest_mode == VOIDmode - || unit <= GET_MODE_BITSIZE (largest_mode))) + || unit <= GET_MODE_BITSIZE (largest_mode)) + && (bitregion_end == 0 + || bitpos - (bitpos % unit) + unit <= bitregion_end + 1)) wide_mode = tmode; } --- gcc/expmed.c.jj 2012-04-19 11:09:13.000000000 +0200 +++ gcc/expmed.c 2012-04-24 17:14:34.264897874 +0200 @@ -640,7 +640,13 @@ store_bit_field_1 (rtx str_rtx, unsigned && !(MEM_P (op0) && MEM_VOLATILE_P (op0) && flag_strict_volatile_bitfields > 0) && ! ((REG_P (op0) || GET_CODE (op0) == SUBREG) - && (bitsize + bitpos > GET_MODE_BITSIZE (op_mode)))) + && (bitsize + bitpos > GET_MODE_BITSIZE (op_mode))) + /* Do not use insv if the bit region is restricted and + op_mode integer at offset doesn't fit into the + restricted region. */ + && !(MEM_P (op0) && bitregion_end + && bitnum - bitpos + GET_MODE_BITSIZE (op_mode) + > bitregion_end + 1)) { struct expand_operand ops[4]; int xbitpos = bitpos; @@ -760,7 +766,7 @@ store_bit_field_1 (rtx str_rtx, unsigned || GET_MODE_BITSIZE (GET_MODE (op0)) > maxbits || (op_mode != MAX_MACHINE_MODE && GET_MODE_SIZE (GET_MODE (op0)) > GET_MODE_SIZE (op_mode))) - bestmode = get_best_mode (bitsize, bitnum, + bestmode = get_best_mode (bitsize, bitnum, bitregion_start, bitregion_end, MEM_ALIGN (op0), (op_mode == MAX_MACHINE_MODE @@ -1096,6 +1102,16 @@ store_split_bit_field (rtx op0, unsigned offset = (bitpos + bitsdone) / unit; thispos = (bitpos + bitsdone) % unit; + /* When region of bytes we can touch is restricted, decrease + UNIT close to the end of the region as needed. */ + if (bitregion_end + && unit > BITS_PER_UNIT + && bitpos + bitsdone - thispos + unit > bitregion_end + 1) + { + unit = unit / 2; + continue; + } + /* THISSIZE must not overrun a word boundary. Otherwise, store_fixed_bit_field will call us again, and we will mutually recurse forever. */ --- gcc/testsuite/gcc.c-torture/compile/pr52979-1.c.jj 2012-04-25 08:27:48.188902802 +0200 +++ gcc/testsuite/gcc.c-torture/compile/pr52979-1.c 2012-04-25 08:27:29.000000000 +0200 @@ -0,0 +1,15 @@ +/* PR middle-end/52979 */ + +struct S +{ + unsigned int a : 16, b : 16, c : 16, d : 16, e : 14; + unsigned int f : 4, g : 14, h : 8; + char i; + int j; +}; + +void +foo (struct S *s) +{ + s->f = 1; +} --- gcc/testsuite/gcc.c-torture/execute/pr52979-1.c.jj 2012-04-24 17:20:51.246662745 +0200 +++ gcc/testsuite/gcc.c-torture/execute/pr52979-1.c 2012-04-24 17:21:03.526586921 +0200 @@ -0,0 +1,40 @@ +/* PR middle-end/52979 */ + +extern void abort (void); +int c, d, e; + +void +foo (void) +{ +} + +struct __attribute__((packed)) S { int g : 31; int h : 6; }; +struct S a = { 1 }; +static struct S b = { 1 }; + +void +bar (void) +{ + a.h = 1; + struct S f = { }; + b = f; + e = 0; + if (d) + c = a.g; +} + +void +baz (void) +{ + bar (); + a = b; +} + +int +main () +{ + baz (); + if (a.g) + abort (); + return 0; +} --- gcc/testsuite/gcc.c-torture/execute/pr52979-2.c.jj 2012-04-24 17:20:54.603642467 +0200 +++ gcc/testsuite/gcc.c-torture/execute/pr52979-2.c 2012-04-24 17:19:15.000000000 +0200 @@ -0,0 +1,40 @@ +/* PR middle-end/52979 */ + +extern void abort (void); +int c, d, e; + +void +foo (void) +{ +} + +struct __attribute__((packed)) S { int g : 31; int h : 6; }; +static struct S b = { 1 }; +struct S a = { 1 }; + +void +bar (void) +{ + a.h = 1; + struct S f = { }; + b = f; + e = 0; + if (d) + c = a.g; +} + +void +baz (void) +{ + bar (); + a = b; +} + +int +main () +{ + baz (); + if (a.g) + abort (); + return 0; +}