From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2153) id DB5C3388A837; Tue, 20 Apr 2021 23:33:39 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org DB5C3388A837 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Jakub Jelinek To: gcc-cvs@gcc.gnu.org Subject: [gcc r9-9434] c: Fix up -Wunused-but-set-* warnings for _Atomics [PR99588] X-Act-Checkin: gcc X-Git-Author: Jakub Jelinek X-Git-Refname: refs/heads/releases/gcc-9 X-Git-Oldrev: 4eb2e3eb0f4a28fade00c1dca626cc947d20e7c4 X-Git-Newrev: 803a95e2a0134105dd259d7ccd258744e94c3233 Message-Id: <20210420233339.DB5C3388A837@sourceware.org> Date: Tue, 20 Apr 2021 23:33:39 +0000 (GMT) X-BeenThere: gcc-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Apr 2021 23:33:40 -0000 https://gcc.gnu.org/g:803a95e2a0134105dd259d7ccd258744e94c3233 commit r9-9434-g803a95e2a0134105dd259d7ccd258744e94c3233 Author: Jakub Jelinek Date: Fri Mar 19 22:54:31 2021 +0100 c: Fix up -Wunused-but-set-* warnings for _Atomics [PR99588] As the following testcases show, compared to -D_Atomic= case we have many -Wunused-but-set-* warning false positives. When an _Atomic variable/parameter is read, we call mark_exp_read on it in convert_lvalue_to_rvalue, but build_atomic_assign does not. For consistency with the non-_Atomic case where we mark_exp_read the lhs for lhs op= ... but not for lhs = ..., this patch does that too. But furthermore we need to pattern match the trees emitted by _Atomic store, so that _Atomic store itself is not marked as being a variable read, but when the result of the store is used, we mark it. 2021-03-19 Jakub Jelinek PR c/99588 * c-typeck.c (mark_exp_read): Recognize what build_atomic_assign with modifycode NOP_EXPR produces and mark the _Atomic var as read if found. (build_atomic_assign): For modifycode of NOP_EXPR, use COMPOUND_EXPRs rather than STATEMENT_LIST. Otherwise call mark_exp_read on lhs. Set TREE_SIDE_EFFECTS on the TARGET_EXPR. * gcc.dg/Wunused-var-5.c: New test. * gcc.dg/Wunused-var-6.c: New test. (cherry picked from commit b1fc1f1c4b2e9005c40ed476b067577da2d2ce84) Diff: --- gcc/c/c-typeck.c | 66 ++++++++++++++++++++++++++++++++---- gcc/testsuite/gcc.dg/Wunused-var-5.c | 23 +++++++++++++ gcc/testsuite/gcc.dg/Wunused-var-6.c | 14 ++++++++ 3 files changed, 97 insertions(+), 6 deletions(-) diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c index e946abee6d5..d97363428d4 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -1939,6 +1939,50 @@ mark_exp_read (tree exp) mark_exp_read (TREE_OPERAND (exp, 0)); break; case COMPOUND_EXPR: + /* Pattern match what build_atomic_assign produces with modifycode + NOP_EXPR. */ + if (VAR_P (TREE_OPERAND (exp, 1)) + && DECL_ARTIFICIAL (TREE_OPERAND (exp, 1)) + && TREE_CODE (TREE_OPERAND (exp, 0)) == COMPOUND_EXPR) + { + tree t1 = TREE_OPERAND (TREE_OPERAND (exp, 0), 0); + tree t2 = TREE_OPERAND (TREE_OPERAND (exp, 0), 1); + if (TREE_CODE (t1) == TARGET_EXPR + && TARGET_EXPR_SLOT (t1) == TREE_OPERAND (exp, 1) + && TREE_CODE (t2) == CALL_EXPR) + { + tree fndecl = get_callee_fndecl (t2); + tree arg = NULL_TREE; + if (fndecl + && TREE_CODE (fndecl) == FUNCTION_DECL + && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL) + && call_expr_nargs (t2) >= 2) + switch (DECL_FUNCTION_CODE (fndecl)) + { + case BUILT_IN_ATOMIC_STORE: + arg = CALL_EXPR_ARG (t2, 1); + break; + case BUILT_IN_ATOMIC_STORE_1: + case BUILT_IN_ATOMIC_STORE_2: + case BUILT_IN_ATOMIC_STORE_4: + case BUILT_IN_ATOMIC_STORE_8: + case BUILT_IN_ATOMIC_STORE_16: + arg = CALL_EXPR_ARG (t2, 0); + break; + default: + break; + } + if (arg) + { + STRIP_NOPS (arg); + if (TREE_CODE (arg) == ADDR_EXPR + && DECL_P (TREE_OPERAND (arg, 0)) + && TYPE_ATOMIC (TREE_TYPE (TREE_OPERAND (arg, 0)))) + mark_exp_read (TREE_OPERAND (arg, 0)); + } + } + } + /* FALLTHRU */ case C_MAYBE_CONST_EXPR: mark_exp_read (TREE_OPERAND (exp, 1)); break; @@ -4031,7 +4075,7 @@ build_atomic_assign (location_t loc, tree lhs, enum tree_code modifycode, vec *params; tree val, nonatomic_lhs_type, nonatomic_rhs_type, newval, newval_addr; tree old, old_addr; - tree compound_stmt; + tree compound_stmt = NULL_TREE; tree stmt, goto_stmt; tree loop_label, loop_decl, done_label, done_decl; @@ -4052,7 +4096,15 @@ build_atomic_assign (location_t loc, tree lhs, enum tree_code modifycode, /* Create a compound statement to hold the sequence of statements with a loop. */ - compound_stmt = c_begin_compound_stmt (false); + if (modifycode != NOP_EXPR) + { + compound_stmt = c_begin_compound_stmt (false); + + /* For consistency with build_modify_expr on non-_Atomic, + mark the lhs as read. Also, it would be very hard to match + such expressions in mark_exp_read. */ + mark_exp_read (lhs); + } /* Remove any excess precision (which is only present here in the case of compound assignments). */ @@ -4078,13 +4130,16 @@ build_atomic_assign (location_t loc, tree lhs, enum tree_code modifycode, TREE_NO_WARNING (val) = 1; rhs = build4 (TARGET_EXPR, nonatomic_rhs_type, val, rhs, NULL_TREE, NULL_TREE); + TREE_SIDE_EFFECTS (rhs) = 1; SET_EXPR_LOCATION (rhs, loc); - add_stmt (rhs); + if (modifycode != NOP_EXPR) + add_stmt (rhs); /* NOP_EXPR indicates it's a straight store of the RHS. Simply issue an atomic_store. */ if (modifycode == NOP_EXPR) { + compound_stmt = rhs; /* Build __atomic_store (&lhs, &val, SEQ_CST) */ rhs = build_unary_op (loc, ADDR_EXPR, val, false); fndecl = builtin_decl_explicit (BUILT_IN_ATOMIC_STORE); @@ -4092,10 +4147,9 @@ build_atomic_assign (location_t loc, tree lhs, enum tree_code modifycode, params->quick_push (rhs); params->quick_push (seq_cst); func_call = c_build_function_call_vec (loc, vNULL, fndecl, params, NULL); - add_stmt (func_call); - /* Finish the compound statement. */ - compound_stmt = c_end_compound_stmt (loc, compound_stmt, false); + compound_stmt = build2 (COMPOUND_EXPR, void_type_node, + compound_stmt, func_call); /* VAL is the value which was stored, return a COMPOUND_STMT of the statement and that value. */ diff --git a/gcc/testsuite/gcc.dg/Wunused-var-5.c b/gcc/testsuite/gcc.dg/Wunused-var-5.c new file mode 100644 index 00000000000..cc5bbf51ab0 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wunused-var-5.c @@ -0,0 +1,23 @@ +/* PR c/99588 */ +/* { dg-do compile } */ +/* { dg-options "-std=c11 -Wunused-but-set-variable" } */ + +void bar (int, ...); +void f1 (void) { static _Atomic int x = 0; bar (0, x); } +void f2 (void) { static _Atomic int x = 0; bar (0, x += 1); } +void f3 (void) { static _Atomic int x = 0; bar (x); } +void f4 (void) { static _Atomic int x = 0; bar (x += 1); } +void f5 (void) { static _Atomic int x = 0; bar (x = 1); } +void f6 (void) { static _Atomic int x = 0; x = 1; } /* { dg-warning "variable 'x' set but not used" } */ +void f7 (void) { static _Atomic int x = 0; x += 3; } +void f8 (void) { _Atomic int x = 0; bar (0, x); } +void f9 (void) { _Atomic int x = 0; bar (0, x += 1); } +void f10 (void) { _Atomic int x = 0; bar (x); } +void f11 (void) { _Atomic int x = 0; bar (x += 1); } +void f12 (void) { _Atomic int x = 0; bar (x = 1); } +void f13 (void) { _Atomic int x = 0; x = 1; } /* { dg-warning "variable 'x' set but not used" } */ +void f14 (void) { _Atomic int x = 0; x += 3; } +void f15 (void) { _Atomic int x = 0; int y = 3; x += y; } +void f16 (void) { _Atomic int x = 0; int y = 3; bar (x += y); } +void f17 (void) { _Atomic int x = 0; int y = 3; x = y; } /* { dg-warning "variable 'x' set but not used" } */ +void f18 (void) { _Atomic int x = 0; int y = 3; bar (x = y); } diff --git a/gcc/testsuite/gcc.dg/Wunused-var-6.c b/gcc/testsuite/gcc.dg/Wunused-var-6.c new file mode 100644 index 00000000000..f48a4554d73 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wunused-var-6.c @@ -0,0 +1,14 @@ +/* PR c/99588 */ +/* { dg-do compile } */ +/* { dg-options "-std=c11 -Wunused-but-set-variable" } */ + +void bar (int, ...); +struct S { int a, b, c; }; +typedef _Atomic struct S T; + +void +foo (void) +{ + static T x = (struct S) { 0, 0, 0 }; /* { dg-bogus "set but not used" } */ + bar (0, x = (struct S) { 1, 1, 1 }); +}