From 56b5041c02bd8ecfddd5e5a373c8a3586a2354ab Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Mon, 7 Aug 2017 20:34:29 +0200 Subject: [PATCH] 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 --- gcc/ChangeLog | 6 ++ gcc/c-family/ChangeLog | 5 + gcc/c-family/c-omp.c | 90 +++++++++++++++-- gcc/gimplify.c | 6 ++ libgomp/ChangeLog | 6 ++ libgomp/testsuite/libgomp.c++/pr69389.C | 3 + libgomp/testsuite/libgomp.c/pr69389.c | 124 ++++++++++++++++++++++++ 7 files changed, 231 insertions(+), 9 deletions(-) create mode 100644 libgomp/testsuite/libgomp.c++/pr69389.C create mode 100644 libgomp/testsuite/libgomp.c/pr69389.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 18dc05d8ffc..e9b90b0ffdf 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,9 @@ +2017-08-07 Jakub Jelinek + + PR c/69389 + * gimplify.c (goa_stabilize_expr): Handle BIT_INSERT_EXPR and + BIT_FIELD_REF. + 2017-08-07 Martin Liska * 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 1243eefa91e..c27da8a3ed2 100644 --- a/gcc/c-family/ChangeLog +++ b/gcc/c-family/ChangeLog @@ -1,3 +1,8 @@ +2017-08-07 Jakub Jelinek + + PR c/69389 + * c-omp.c (c_finish_omp_atomic): Handle atomics on bitfields. + 2017-08-07 Eric Botcazou * 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 977cb0ea153..900f5a33943 100644 --- 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 76a08c67061..ed2ec646da2 100644 --- 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 62d4f27d2f3..f17a17b138d 100644 --- a/libgomp/ChangeLog +++ b/libgomp/ChangeLog @@ -1,3 +1,9 @@ +2017-08-07 Jakub Jelinek + + PR c/69389 + * testsuite/libgomp.c/pr69389.c: New test. + * testsuite/libgomp.c++/pr69389.C: New test. + 2017-08-07 Tom de Vries PR middle-end/78266 diff --git a/libgomp/testsuite/libgomp.c++/pr69389.C b/libgomp/testsuite/libgomp.c++/pr69389.C new file mode 100644 index 00000000000..9fcb0ffc6d0 --- /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 index 00000000000..07faff46f40 --- /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; +} -- 2.30.2