| Message ID | alpine.LNX.2.00.1201161001140.4999@zhemvz.fhfr.qr |
|---|---|
| State | New |
| Headers | show |
On Mon, 16 Jan 2012, Richard Guenther wrote: > On Fri, 13 Jan 2012, Joseph S. Myers wrote: > > > On Fri, 13 Jan 2012, Richard Guenther wrote: > > > > > This fixes PR37985 where since > > > http://gcc.gnu.org/ml/gcc-patches/2006-08/msg01041.html we > > > mark conversions produced by convert_to_integer with TREE_NO_WARNING. > > > This may shadow "real" stmts with no effects, thus we should > > > simply strip them again before checking for TREE_SIDE_EFFECTS. > > > > > > Bootstrap & regtest pending on x86_64-unknown-linux-gnu. > > > > > > Ok if that passes? > > > > OK. > > It FAILs gcc.dg/20040202-1.c (we warn about a folded memcpy). It > looks like wrapping things in TREE_NO_WARNING NOP_EXPRs is > fragile as soon as you consider multiple warning kinds (thus, of > course the patch causing this regression is at fault). > > OTOH for the regression warning on folded stmts I really wonder > why c_process_expr_stmt folds at all before emitting warnings - why > intentionally divert from the source AST here? I'm testing > the following before giving up on this regression (which solves > the gcc.dg/20040202-1.c FAIL at least). Which works besides now FAILing gcc.dg/strncpy-fix-1.c. We get a "value computed is not used" warning because we are still folding the strncpy to a memcpy because we fold all builtin calls at the very point they are created (build_function_call_vec): if (name != NULL_TREE && !strncmp (IDENTIFIER_POINTER (name), "__builtin_", 10)) { if (require_constant_value) result = fold_build_call_array_initializer_loc (loc, TREE_TYPE (fntype), function, nargs, argarray); else result = fold_build_call_array_loc (loc, TREE_TYPE (fntype), function, nargs, argarray); "Fixed" by not doing that if !require_constant_value. We'll fold it during the c_fully_fold call anyway (and during gimplification again). Re-testing as follows. Richard. 2012-01-13 Richard Guenther <rguenther@suse.de> PR c/37985 * c-typeck.c (emit_side_effect_warnings): Strip conversions with TREE_NO_WARNING. (c_process_expr_stmt): Fold the stmt after emitting warnings. (build_function_call_vec): Do not fold calls here if we do not require a constant value. * gcc.dg/Wunused-value-4.c: New testcase. Index: gcc/testsuite/gcc.dg/Wunused-value-4.c =================================================================== *** gcc/testsuite/gcc.dg/Wunused-value-4.c (revision 0) --- gcc/testsuite/gcc.dg/Wunused-value-4.c (revision 0) *************** *** 0 **** --- 1,9 ---- + /* PR c/37985 */ + /* { dg-do compile } */ + /* { dg-options "-Wunused-value" } */ + + unsigned char foo(unsigned char a) + { + a >> 2; /* { dg-warning "statement with no effect" } */ + return a; + } Index: gcc/c-typeck.c =================================================================== *** gcc/c-typeck.c (revision 183205) --- gcc/c-typeck.c (working copy) *************** build_function_call_vec (location_t loc, *** 2826,2841 **** /* Check that the arguments to the function are valid. */ check_function_arguments (fntype, nargs, argarray); ! if (name != NULL_TREE && !strncmp (IDENTIFIER_POINTER (name), "__builtin_", 10)) { ! if (require_constant_value) ! result = ! fold_build_call_array_initializer_loc (loc, TREE_TYPE (fntype), ! function, nargs, argarray); ! else ! result = fold_build_call_array_loc (loc, TREE_TYPE (fntype), ! function, nargs, argarray); if (TREE_CODE (result) == NOP_EXPR && TREE_CODE (TREE_OPERAND (result, 0)) == INTEGER_CST) STRIP_TYPE_NOPS (result); --- 2826,2838 ---- /* Check that the arguments to the function are valid. */ check_function_arguments (fntype, nargs, argarray); ! if (require_constant_value ! && name != NULL_TREE && !strncmp (IDENTIFIER_POINTER (name), "__builtin_", 10)) { ! result = ! fold_build_call_array_initializer_loc (loc, TREE_TYPE (fntype), ! function, nargs, argarray); if (TREE_CODE (result) == NOP_EXPR && TREE_CODE (TREE_OPERAND (result, 0)) == INTEGER_CST) STRIP_TYPE_NOPS (result); *************** c_finish_bc_stmt (location_t loc, tree * *** 9163,9168 **** --- 9160,9169 ---- static void emit_side_effect_warnings (location_t loc, tree expr) { + /* Strip conversions marked with TREE_NO_WARNING. */ + while (TREE_NO_WARNING (expr) && CONVERT_EXPR_P (expr)) + expr = TREE_OPERAND (expr, 0); + if (expr == error_mark_node) ; else if (!TREE_SIDE_EFFECTS (expr)) *************** c_process_expr_stmt (location_t loc, tre *** 9186,9193 **** if (!expr) return NULL_TREE; - expr = c_fully_fold (expr, false, NULL); - if (warn_sequence_point) verify_sequence_points (expr); --- 9187,9192 ---- *************** c_process_expr_stmt (location_t loc, tre *** 9213,9218 **** --- 9212,9219 ---- || TREE_CODE (exprv) == ADDR_EXPR) mark_exp_read (exprv); + expr = c_fully_fold (expr, false, NULL); + /* If the expression is not of a type to which we cannot assign a line number, wrap the thing in a no-op NOP_EXPR. */ if (DECL_P (expr) || CONSTANT_CLASS_P (expr))
On Mon, 16 Jan 2012, Richard Guenther wrote: > On Mon, 16 Jan 2012, Richard Guenther wrote: > > > On Fri, 13 Jan 2012, Joseph S. Myers wrote: > > > > > On Fri, 13 Jan 2012, Richard Guenther wrote: > > > > > > > This fixes PR37985 where since > > > > http://gcc.gnu.org/ml/gcc-patches/2006-08/msg01041.html we > > > > mark conversions produced by convert_to_integer with TREE_NO_WARNING. > > > > This may shadow "real" stmts with no effects, thus we should > > > > simply strip them again before checking for TREE_SIDE_EFFECTS. > > > > > > > > Bootstrap & regtest pending on x86_64-unknown-linux-gnu. > > > > > > > > Ok if that passes? > > > > > > OK. > > > > It FAILs gcc.dg/20040202-1.c (we warn about a folded memcpy). It > > looks like wrapping things in TREE_NO_WARNING NOP_EXPRs is > > fragile as soon as you consider multiple warning kinds (thus, of > > course the patch causing this regression is at fault). > > > > OTOH for the regression warning on folded stmts I really wonder > > why c_process_expr_stmt folds at all before emitting warnings - why > > intentionally divert from the source AST here? I'm testing > > the following before giving up on this regression (which solves > > the gcc.dg/20040202-1.c FAIL at least). > > Which works besides now FAILing gcc.dg/strncpy-fix-1.c. We get > a "value computed is not used" warning because we are still folding > the strncpy to a memcpy because we fold all builtin calls at the very > point they are created (build_function_call_vec): > > if (name != NULL_TREE > && !strncmp (IDENTIFIER_POINTER (name), "__builtin_", 10)) > { > if (require_constant_value) > result = > fold_build_call_array_initializer_loc (loc, TREE_TYPE (fntype), > function, nargs, > argarray); > else > result = fold_build_call_array_loc (loc, TREE_TYPE (fntype), > function, nargs, argarray); > > "Fixed" by not doing that if !require_constant_value. We'll fold > it during the c_fully_fold call anyway (and during gimplification again). > > Re-testing as follows. Which does not work either - we seem to rely on that early folding at least for the type-generic math stuff (gcc.dg/c99-tgmath-*.c). I'm dropping all these patches. There doesn't seem to be a good way to make everything well-behaved with the patch causing the regression installed. Richard.
Index: gcc/c-typeck.c =================================================================== --- gcc/c-typeck.c (revision 183205) +++ gcc/c-typeck.c (working copy) @@ -9163,6 +9163,10 @@ c_finish_bc_stmt (location_t loc, tree * static void emit_side_effect_warnings (location_t loc, tree expr) { + /* Strip conversions marked with TREE_NO_WARNING. */ + while (TREE_NO_WARNING (expr) && CONVERT_EXPR_P (expr)) + expr = TREE_OPERAND (expr, 0); + if (expr == error_mark_node) ; else if (!TREE_SIDE_EFFECTS (expr)) @@ -9186,8 +9190,6 @@ c_process_expr_stmt (location_t loc, tre if (!expr) return NULL_TREE; - expr = c_fully_fold (expr, false, NULL); - if (warn_sequence_point) verify_sequence_points (expr); @@ -9213,6 +9215,8 @@ c_process_expr_stmt (location_t loc, tre || TREE_CODE (exprv) == ADDR_EXPR) mark_exp_read (exprv); + expr = c_fully_fold (expr, false, NULL); + /* If the expression is not of a type to which we cannot assign a line number, wrap the thing in a no-op NOP_EXPR. */ if (DECL_P (expr) || CONSTANT_CLASS_P (expr))