[committed] Add support for OpenMP atomics on bitfields (PR c/69389)

Submitted by Jakub Jelinek on Aug. 7, 2017, 6:36 p.m.

Details

Message ID 20170807183645.GO2123@tucnak
State New
Headers show

Commit Message

Jakub Jelinek Aug. 7, 2017, 6:36 p.m.
Hi!

Apparently there is no restriction on #pragma omp atomic not being done on a
bitfield, so this patch handles it by performing atomics on
DECL_BIT_FIELD_REPRESENTATIVE instead.

Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.

2017-08-07  Jakub Jelinek  <jakub@redhat.com>

	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.


	Jakub

Patch hide | download patch | download mbox

--- gcc/gimplify.c.jj	2017-08-01 10:21:05.000000000 +0200
+++ gcc/gimplify.c	2017-08-07 15:20:46.236479909 +0200
@@ -10825,6 +10825,7 @@  goa_stabilize_expr (tree *expr_p, gimple
 	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
 	  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;
     }
--- gcc/c-family/c-omp.c.jj	2017-07-27 21:12:57.000000000 +0200
+++ gcc/c-family/c-omp.c	2017-08-07 17:06:09.870769403 +0200
@@ -181,6 +181,7 @@  c_finish_omp_atomic (location_t loc, enu
 		     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, enu
     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, enu
       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, enu
   /* 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, enu
   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, enu
      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, enu
       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, enu
 	    }
 	}
     }
-  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)
--- libgomp/testsuite/libgomp.c/pr69389.c.jj	2017-08-07 17:21:30.051788530 +0200
+++ libgomp/testsuite/libgomp.c/pr69389.c	2017-08-07 17:21:15.000000000 +0200
@@ -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;
+}
--- libgomp/testsuite/libgomp.c++/pr69389.C.jj	2017-08-07 17:22:08.065334493 +0200
+++ libgomp/testsuite/libgomp.c++/pr69389.C	2017-08-07 17:22:02.279403600 +0200
@@ -0,0 +1,3 @@ 
+// PR c/69389
+
+#include "../libgomp.c/pr69389.c"