re PR c/69389 (bit field incompatible with OpenMP atomic update)

	PR c/69389
	* gimplify.c (goa_stabilize_expr): Handle BIT_INSERT_EXPR and
	BIT_FIELD_REF.

	* c-omp.c (c_finish_omp_atomic): Handle atomics on bitfields.

	* testsuite/libgomp.c/pr69389.c: New test.
	* testsuite/libgomp.c++/pr69389.C: New test.

From-SVN: r250929

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 18dc05d8ffc204b875eaa52a27ed4937208870cc..e9b90b0ffdf57b509e098aa176c7a1e3af29715e 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2017-08-07  Jakub Jelinek  <jakub@redhat.com>
+
+       PR c/69389
+       * gimplify.c (goa_stabilize_expr): Handle BIT_INSERT_EXPR and
+       BIT_FIELD_REF.
+
 2017-08-07  Martin Liska  <mliska@suse.cz>
 
        * config/m32c/m32c.c: Add include of stringpool.h and attribs.h.

diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog
index 1243eefa91edee83d1886956dbbae0ad708626ea..c27da8a3ed20e8621ca32155320804ed1e1d99fc 100644 (file)
--- a/gcc/c-family/ChangeLog
+++ b/gcc/c-family/ChangeLog
@@ -1,3 +1,8 @@
+2017-08-07  Jakub Jelinek  <jakub@redhat.com>
+
+       PR c/69389
+       * c-omp.c (c_finish_omp_atomic): Handle atomics on bitfields.
+
 2017-08-07  Eric Botcazou  <ebotcazou@adacore.com>
 
        * c-ada-spec.c (has_nontrivial_methods): Test for FUNCTION_DECL.

diff --git a/gcc/c-family/c-omp.c b/gcc/c-family/c-omp.c
index 977cb0ea15303d22486769541faf070cfcb91002..900f5a33943b4ad8abe3c9b3ab7bde8a9b6a2afd 100644 (file)
--- a/gcc/c-family/c-omp.c
+++ b/gcc/c-family/c-omp.c
@@ -181,6 +181,7 @@ c_finish_omp_atomic (location_t loc, enum tree_code code,
                     bool test)
 {
   tree x, type, addr, pre = NULL_TREE;
+  HOST_WIDE_INT bitpos = 0, bitsize = 0;
 
   if (lhs == error_mark_node || rhs == error_mark_node
       || v == error_mark_node || lhs1 == error_mark_node
@@ -209,6 +210,29 @@ c_finish_omp_atomic (location_t loc, enum tree_code code,
     opcode = TRUNC_DIV_EXPR;
 
   /* ??? Validate that rhs does not overlap lhs.  */
+  tree blhs = NULL;
+  if (TREE_CODE (lhs) == COMPONENT_REF
+      && TREE_CODE (TREE_OPERAND (lhs, 1)) == FIELD_DECL
+      && DECL_C_BIT_FIELD (TREE_OPERAND (lhs, 1))
+      && DECL_BIT_FIELD_REPRESENTATIVE (TREE_OPERAND (lhs, 1)))
+    {
+      tree field = TREE_OPERAND (lhs, 1);
+      tree repr = DECL_BIT_FIELD_REPRESENTATIVE (field);
+      if (tree_fits_uhwi_p (DECL_FIELD_OFFSET (field))
+         && tree_fits_uhwi_p (DECL_FIELD_OFFSET (repr)))
+       bitpos = (tree_to_uhwi (DECL_FIELD_OFFSET (field))
+                 - tree_to_uhwi (DECL_FIELD_OFFSET (repr))) * BITS_PER_UNIT;
+      else
+       bitpos = 0;
+      bitpos += (tree_to_uhwi (DECL_FIELD_BIT_OFFSET (field))
+                - tree_to_uhwi (DECL_FIELD_BIT_OFFSET (repr)));
+      gcc_assert (tree_fits_shwi_p (DECL_SIZE (field)));
+      bitsize = tree_to_shwi (DECL_SIZE (field));
+      blhs = lhs;
+      type = TREE_TYPE (repr);
+      lhs = build3 (COMPONENT_REF, TREE_TYPE (repr), TREE_OPERAND (lhs, 0),
+                   repr, TREE_OPERAND (lhs, 2));
+    }
 
   /* Take and save the address of the lhs.  From then on we'll reference it
      via indirection.  */
@@ -228,13 +252,18 @@ c_finish_omp_atomic (location_t loc, enum tree_code code,
       DECL_CONTEXT (var) = current_function_decl;
       addr = build4 (TARGET_EXPR, TREE_TYPE (addr), var, addr, NULL, NULL);
     }
+  tree orig_lhs = lhs;
   lhs = build_indirect_ref (loc, addr, RO_NULL);
+  tree new_lhs = lhs;
 
   if (code == OMP_ATOMIC_READ)
     {
       x = build1 (OMP_ATOMIC_READ, type, addr);
       SET_EXPR_LOCATION (x, loc);
       OMP_ATOMIC_SEQ_CST (x) = seq_cst;
+      if (blhs)
+       x = build3_loc (loc, BIT_FIELD_REF, TREE_TYPE (blhs), x,
+                       bitsize_int (bitsize), bitsize_int (bitpos));
       return build_modify_expr (loc, v, NULL_TREE, NOP_EXPR,
                                loc, x, NULL_TREE);
     }
@@ -242,14 +271,25 @@ c_finish_omp_atomic (location_t loc, enum tree_code code,
   /* There are lots of warnings, errors, and conversions that need to happen
      in the course of interpreting a statement.  Use the normal mechanisms
      to do this, and then take it apart again.  */
-  if (swapped)
+  if (blhs)
+    {
+      lhs = build3_loc (loc, BIT_FIELD_REF, TREE_TYPE (blhs), lhs,
+                       bitsize_int (bitsize), bitsize_int (bitpos));
+      if (swapped)
+       rhs = build_binary_op (loc, opcode, rhs, lhs, 1);
+      else if (opcode != NOP_EXPR)
+       rhs = build_binary_op (loc, opcode, lhs, rhs, 1);
+      opcode = NOP_EXPR;
+    }
+  else if (swapped)
     {
       rhs = build_binary_op (loc, opcode, rhs, lhs, 1);
       opcode = NOP_EXPR;
     }
   bool save = in_late_binary_op;
   in_late_binary_op = true;
-  x = build_modify_expr (loc, lhs, NULL_TREE, opcode, loc, rhs, NULL_TREE);
+  x = build_modify_expr (loc, blhs ? blhs : lhs, NULL_TREE, opcode,
+                        loc, rhs, NULL_TREE);
   in_late_binary_op = save;
   if (x == error_mark_node)
     return error_mark_node;
@@ -262,6 +302,10 @@ c_finish_omp_atomic (location_t loc, enum tree_code code,
   gcc_assert (TREE_CODE (x) == MODIFY_EXPR);
   rhs = TREE_OPERAND (x, 1);
 
+  if (blhs)
+    rhs = build3_loc (loc, BIT_INSERT_EXPR, type, new_lhs,
+                     rhs, bitsize_int (bitpos));
+
   /* Punt the actual generation of atomic operations to common code.  */
   if (code == OMP_ATOMIC)
     type = void_type_node;
@@ -273,8 +317,8 @@ c_finish_omp_atomic (location_t loc, enum tree_code code,
      location, just diagnose different variables.  */
   if (rhs1
       && VAR_P (rhs1)
-      && VAR_P (lhs)
-      && rhs1 != lhs
+      && VAR_P (orig_lhs)
+      && rhs1 != orig_lhs
       && !test)
     {
       if (code == OMP_ATOMIC)
@@ -286,29 +330,57 @@ c_finish_omp_atomic (location_t loc, enum tree_code code,
       return error_mark_node;
     }
 
+  if (lhs1
+      && lhs1 != orig_lhs
+      && TREE_CODE (lhs1) == COMPONENT_REF
+      && TREE_CODE (TREE_OPERAND (lhs1, 1)) == FIELD_DECL
+      && DECL_C_BIT_FIELD (TREE_OPERAND (lhs1, 1))
+      && DECL_BIT_FIELD_REPRESENTATIVE (TREE_OPERAND (lhs1, 1)))
+    {
+      tree field = TREE_OPERAND (lhs1, 1);
+      tree repr = DECL_BIT_FIELD_REPRESENTATIVE (field);
+      lhs1 = build3 (COMPONENT_REF, TREE_TYPE (repr), TREE_OPERAND (lhs1, 0),
+                    repr, TREE_OPERAND (lhs1, 2));
+    }
+  if (rhs1
+      && rhs1 != orig_lhs
+      && TREE_CODE (rhs1) == COMPONENT_REF
+      && TREE_CODE (TREE_OPERAND (rhs1, 1)) == FIELD_DECL
+      && DECL_C_BIT_FIELD (TREE_OPERAND (rhs1, 1))
+      && DECL_BIT_FIELD_REPRESENTATIVE (TREE_OPERAND (rhs1, 1)))
+    {
+      tree field = TREE_OPERAND (rhs1, 1);
+      tree repr = DECL_BIT_FIELD_REPRESENTATIVE (field);
+      rhs1 = build3 (COMPONENT_REF, TREE_TYPE (repr), TREE_OPERAND (rhs1, 0),
+                    repr, TREE_OPERAND (rhs1, 2));
+    }
+
   if (code != OMP_ATOMIC)
     {
       /* Generally it is hard to prove lhs1 and lhs are the same memory
         location, just diagnose different variables.  */
-      if (lhs1 && VAR_P (lhs1) && VAR_P (lhs))
+      if (lhs1 && VAR_P (lhs1) && VAR_P (orig_lhs))
        {
-         if (lhs1 != lhs && !test)
+         if (lhs1 != orig_lhs && !test)
            {
              error_at (loc, "%<#pragma omp atomic capture%> uses two "
                             "different variables for memory");
              return error_mark_node;
            }
        }
+      if (blhs)
+       x = build3_loc (loc, BIT_FIELD_REF, TREE_TYPE (blhs), x,
+                       bitsize_int (bitsize), bitsize_int (bitpos));
       x = build_modify_expr (loc, v, NULL_TREE, NOP_EXPR,
                             loc, x, NULL_TREE);
-      if (rhs1 && rhs1 != lhs)
+      if (rhs1 && rhs1 != orig_lhs)
        {
          tree rhs1addr = build_unary_op (loc, ADDR_EXPR, rhs1, false);
          if (rhs1addr == error_mark_node)
            return error_mark_node;
          x = omit_one_operand_loc (loc, type, x, rhs1addr);
        }
-      if (lhs1 && lhs1 != lhs)
+      if (lhs1 && lhs1 != orig_lhs)
        {
          tree lhs1addr = build_unary_op (loc, ADDR_EXPR, lhs1, false);
          if (lhs1addr == error_mark_node)
@@ -323,7 +395,7 @@ c_finish_omp_atomic (location_t loc, enum tree_code code,
            }
        }
     }
-  else if (rhs1 && rhs1 != lhs)
+  else if (rhs1 && rhs1 != orig_lhs)
     {
       tree rhs1addr = build_unary_op (loc, ADDR_EXPR, rhs1, false);
       if (rhs1addr == error_mark_node)

diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 76a08c67061e85c1004d6852e21470ebe63e41e1..ed2ec646da2ec451f5879d62c5b7fab01fa6cb44 100644 (file)
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -10825,6 +10825,7 @@ goa_stabilize_expr (tree *expr_p, gimple_seq *pre_p, tree lhs_addr,
        case TRUTH_AND_EXPR:
        case TRUTH_OR_EXPR:
        case TRUTH_XOR_EXPR:
+       case BIT_INSERT_EXPR:
          saw_lhs |= goa_stabilize_expr (&TREE_OPERAND (expr, 1), pre_p,
                                         lhs_addr, lhs_var);
          /* FALLTHRU */
@@ -10843,6 +10844,11 @@ goa_stabilize_expr (tree *expr_p, gimple_seq *pre_p, tree lhs_addr,
          break;
        }
       break;
+    case tcc_reference:
+      if (TREE_CODE (expr) == BIT_FIELD_REF)
+       saw_lhs |= goa_stabilize_expr (&TREE_OPERAND (expr, 0), pre_p,
+                                      lhs_addr, lhs_var);
+      break;
     default:
       break;
     }

diff --git a/libgomp/ChangeLog b/libgomp/ChangeLog
index 62d4f27d2f34251b3edcd54ce350bcbe3bb0ea1e..f17a17b138d8b29a336665b589d8bf9c05a598fb 100644 (file)
--- a/libgomp/ChangeLog
+++ b/libgomp/ChangeLog
@@ -1,3 +1,9 @@
+2017-08-07  Jakub Jelinek  <jakub@redhat.com>
+
+       PR c/69389
+       * testsuite/libgomp.c/pr69389.c: New test.
+       * testsuite/libgomp.c++/pr69389.C: New test.
+
 2017-08-07  Tom de Vries  <tom@codesourcery.com>
 
        PR middle-end/78266

diff --git a/libgomp/testsuite/libgomp.c++/pr69389.C b/libgomp/testsuite/libgomp.c++/pr69389.C
new file mode 100644 (file)
index 0000000..9fcb0ff
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c++/pr69389.C
@@ -0,0 +1,3 @@
+// PR c/69389
+
+#include "../libgomp.c/pr69389.c"

diff --git a/libgomp/testsuite/libgomp.c/pr69389.c b/libgomp/testsuite/libgomp.c/pr69389.c
new file mode 100644 (file)
index 0000000..07faff4
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/pr69389.c
@@ -0,0 +1,124 @@
+/* PR c/69389 */
+
+struct S { unsigned int a : 10; unsigned int b : 4; unsigned int c : 18; } s = { 210, 11, 1235 };
+
+unsigned int
+f1 (void)
+{
+  unsigned int v;
+  #pragma omp atomic read
+  v = s.b;
+  return v;
+}
+
+void
+f2 (unsigned int v)
+{
+  #pragma omp atomic write
+  s.b = v;
+}
+
+void
+f3 (void)
+{
+  #pragma omp atomic
+  s.b |= 1;
+}
+
+int
+f4 (void)
+{
+  int v;
+  #pragma omp atomic capture
+  v = s.b += 8;
+  return v;
+}
+
+int
+f5 (void)
+{
+  int v;
+  #pragma omp atomic capture
+  {
+    v = s.b;
+    s.b -= 4;
+  }
+  return v;
+}
+
+void
+f6 (void)
+{
+  #pragma omp atomic
+  s.b = s.b & 7;
+}
+
+void
+f7 (void)
+{
+  #pragma omp atomic
+  s.b = ~7 & s.b;
+}
+
+int
+f8 (void)
+{
+  int v;
+  #pragma omp atomic capture
+  v = s.b = 8 + s.b;
+  return v;
+}
+
+int
+f9 (void)
+{
+  int v;
+  #pragma omp atomic capture
+  {
+    v = s.b;
+    s.b = s.b - 4;
+  }
+  return v;
+}
+
+int
+main ()
+{
+  if (f1 () != 11)
+    __builtin_abort ();
+  f2 (4);
+  if (s.a != 210 || s.b != 4 || s.c != 1235)
+    __builtin_abort ();
+  s.a = 813;
+  s.c = 31532;
+  if (f1 () != 4)
+    __builtin_abort ();
+  f3 ();
+  if (f1 () != 5)
+    __builtin_abort ();
+  if (s.a != 813 || s.b != 5 || s.c != 31532)
+    __builtin_abort ();
+  if (f4 () != 13)
+    __builtin_abort ();
+  if (f1 () != 13)
+    __builtin_abort ();
+  f2 (14);
+  if (s.a != 813 || s.b != 14 || s.c != 31532)
+    __builtin_abort ();
+  if (f5 () != 14)
+    __builtin_abort ();
+  if (f1 () != 10 || s.a != 813 || s.b != 10 || s.c != 31532)
+    __builtin_abort ();
+  f6 ();
+  if (f1 () != 2)
+    __builtin_abort ();
+  f2 (15);
+  f7 ();
+  if (f1 () != 8)
+    __builtin_abort ();
+  if (f8 () != 0 || s.a != 813 || s.b != 0 || s.c != 31532)
+    __builtin_abort ();
+  if (f9 () != 0 || s.a != 813 || s.b != 12 || s.c != 31532)
+    __builtin_abort ();
+  return 0;
+}