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 <ja...@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.
--- 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"
Jakub
