On Wed, May 13, 2015 at 03:06:00PM +0000, Joseph Myers wrote: > > It won't handle e.g. 0 * (unsigned) (1 / 0). > > I think this illustrates why handling this in the folding-for-optimization > is a mistake. > > For optimization, it's perfectly fine to fold away 0 * (something without > side effects), and division by 0 should only be considered to have side > effects if language semantic options were specified to that effect (such > as using ubsan), which should then have the effect of producing GENERIC > that's not a simple division but represents whatever checks are required. > > Rather, how about having an extra argument to c_fully_fold_internal (e.g. > for_int_const) indicating that the folding is of an expression with > integer constant operands (so this argument would be true in the new case > of folding the contents of a C_MAYBE_CONST_EXPR with > C_MAYBE_CONST_EXPR_INT_OPERANDS set)? In that special case, > c_fully_fold_internal would only fold the expression itself if all > evaluated operands folded to INTEGER_CSTs (so if something that doesn't > get folded, such as division by 0, is encountered, then all evaluated > expressions containing it also don't get folded).
Did you mean something like the following? It is on top of the previous c_fully_fold_internal patch; if it makes any sense, I'll squash these into one. With the following, I don't see any regressions in the C dg.exp testsuite. This still doesn't reject enum { A = 0 * (unsigned) (1 / 0) };, but note that we don't reject such an enum at present. Thanks. diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c index 24200f0..1dc181d 100644 --- gcc/c-family/c-common.c +++ gcc/c-family/c-common.c @@ -315,7 +315,7 @@ const struct fname_var_t fname_vars[] = /* Global visibility options. */ struct visibility_flags visibility_options; -static tree c_fully_fold_internal (tree expr, bool, bool *, bool *); +static tree c_fully_fold_internal (tree expr, bool, bool *, bool *, bool); static tree check_case_value (location_t, tree); static bool check_case_bounds (location_t, tree, tree, tree *, tree *); @@ -1148,7 +1148,7 @@ c_fully_fold (tree expr, bool in_init, bool *maybe_const) expr = TREE_OPERAND (expr, 0); } ret = c_fully_fold_internal (expr, in_init, maybe_const, - &maybe_const_itself); + &maybe_const_itself, false); if (eptype) ret = fold_convert_loc (loc, eptype, ret); *maybe_const &= maybe_const_itself; @@ -1161,11 +1161,13 @@ c_fully_fold (tree expr, bool in_init, bool *maybe_const) arithmetic overflow (for C90, *MAYBE_CONST_OPERANDS is carried from both evaluated and unevaluated subexpressions while *MAYBE_CONST_ITSELF is carried from only evaluated - subexpressions). */ + subexpressions). FOR_INT_CONST indicates if EXPR is an expression + with integer constant operands, and if any of the operands doesn't + get folded to an integer constant, don't fold the expression itself. */ static tree c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands, - bool *maybe_const_itself) + bool *maybe_const_itself, bool for_int_const) { tree ret = expr; enum tree_code code = TREE_CODE (expr); @@ -1212,7 +1214,7 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands, { *maybe_const_itself = false; inner = c_fully_fold_internal (inner, in_init, maybe_const_operands, - maybe_const_itself); + maybe_const_itself, true); } if (pre && !in_init) ret = build2 (COMPOUND_EXPR, TREE_TYPE (expr), pre, inner); @@ -1263,7 +1265,7 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands, op1 = TREE_OPERAND (expr, 1); op2 = TREE_OPERAND (expr, 2); op0 = c_fully_fold_internal (op0, in_init, maybe_const_operands, - maybe_const_itself); + maybe_const_itself, for_int_const); STRIP_TYPE_NOPS (op0); if (op0 != orig_op0) ret = build3 (COMPONENT_REF, TREE_TYPE (expr), op0, op1, op2); @@ -1280,10 +1282,10 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands, op2 = TREE_OPERAND (expr, 2); op3 = TREE_OPERAND (expr, 3); op0 = c_fully_fold_internal (op0, in_init, maybe_const_operands, - maybe_const_itself); + maybe_const_itself, for_int_const); STRIP_TYPE_NOPS (op0); op1 = c_fully_fold_internal (op1, in_init, maybe_const_operands, - maybe_const_itself); + maybe_const_itself, for_int_const); STRIP_TYPE_NOPS (op1); op1 = decl_constant_value_for_optimization (op1); if (op0 != orig_op0 || op1 != orig_op1) @@ -1340,7 +1342,7 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands, orig_op0 = op0 = TREE_OPERAND (expr, 0); orig_op1 = op1 = TREE_OPERAND (expr, 1); op0 = c_fully_fold_internal (op0, in_init, maybe_const_operands, - maybe_const_itself); + maybe_const_itself, for_int_const); STRIP_TYPE_NOPS (op0); if (code != MODIFY_EXPR && code != PREDECREMENT_EXPR @@ -1352,9 +1354,14 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands, expression for the sake of conversion warnings. */ if (code != MODIFY_EXPR) op1 = c_fully_fold_internal (op1, in_init, maybe_const_operands, - maybe_const_itself); + maybe_const_itself, for_int_const); STRIP_TYPE_NOPS (op1); op1 = decl_constant_value_for_optimization (op1); + + if (for_int_const && (TREE_CODE (op0) != INTEGER_CST + || TREE_CODE (op1) != INTEGER_CST)) + goto out; + if (op0 != orig_op0 || op1 != orig_op1 || in_init) ret = in_init ? fold_build2_initializer_loc (loc, code, TREE_TYPE (expr), op0, op1) @@ -1424,7 +1431,7 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands, /* Unary operations. */ orig_op0 = op0 = TREE_OPERAND (expr, 0); op0 = c_fully_fold_internal (op0, in_init, maybe_const_operands, - maybe_const_itself); + maybe_const_itself, for_int_const); STRIP_TYPE_NOPS (op0); if (code != ADDR_EXPR && code != REALPART_EXPR && code != IMAGPART_EXPR) op0 = decl_constant_value_for_optimization (op0); @@ -1472,14 +1479,16 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands, arguments. */ orig_op0 = op0 = TREE_OPERAND (expr, 0); orig_op1 = op1 = TREE_OPERAND (expr, 1); - op0 = c_fully_fold_internal (op0, in_init, &op0_const, &op0_const_self); + op0 = c_fully_fold_internal (op0, in_init, &op0_const, &op0_const_self, + for_int_const); STRIP_TYPE_NOPS (op0); unused_p = (op0 == (code == TRUTH_ANDIF_EXPR ? truthvalue_false_node : truthvalue_true_node)); c_disable_warnings (unused_p); - op1 = c_fully_fold_internal (op1, in_init, &op1_const, &op1_const_self); + op1 = c_fully_fold_internal (op1, in_init, &op1_const, &op1_const_self, + for_int_const); STRIP_TYPE_NOPS (op1); c_enable_warnings (unused_p); @@ -1510,16 +1519,19 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands, orig_op0 = op0 = TREE_OPERAND (expr, 0); orig_op1 = op1 = TREE_OPERAND (expr, 1); orig_op2 = op2 = TREE_OPERAND (expr, 2); - op0 = c_fully_fold_internal (op0, in_init, &op0_const, &op0_const_self); + op0 = c_fully_fold_internal (op0, in_init, &op0_const, &op0_const_self, + for_int_const); STRIP_TYPE_NOPS (op0); c_disable_warnings (op0 == truthvalue_false_node); - op1 = c_fully_fold_internal (op1, in_init, &op1_const, &op1_const_self); + op1 = c_fully_fold_internal (op1, in_init, &op1_const, &op1_const_self, + for_int_const); STRIP_TYPE_NOPS (op1); c_enable_warnings (op0 == truthvalue_false_node); c_disable_warnings (op0 == truthvalue_true_node); - op2 = c_fully_fold_internal (op2, in_init, &op2_const, &op2_const_self); + op2 = c_fully_fold_internal (op2, in_init, &op2_const, &op2_const_self, + for_int_const); STRIP_TYPE_NOPS (op2); c_enable_warnings (op0 == truthvalue_true_node); Marek