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?

2012-04-25  Jakub Jelinek  <ja...@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

Reply via email to