From fc8d9e4497032dd295aac9414042163f92250b77 Mon Sep 17 00:00:00 2001 From: Richard Biener Date: Mon, 4 Apr 2022 12:23:28 +0200 Subject: [PATCH] tree-optimization/105142 - wrong code with maybe_fold_{and,or}_comparisons The following avoids expanding definitions in regions conditionally executed under the condition A when simplifying A && B or A || B. This is done by passing down the basic-block of the outer condition to maybe_fold_{and,or}_comparisons, through the various helpers in gimple-fold.cc that might call back to maybe_fold_{and,or}_comparisons and ultimatively to maybe_fold_comparisons_from_match_pd where the fix is to provide a custom valueization hook to gimple_match_op::resimplify that avoids looking at definitions that do not dominate the outer block. For the testcase this avoids combining a stmt that invokes undefined integer overflow when the outer condition is false but it also aovids combining stmts with range information that is derived from the outer condition. The new parameter to maybe_fold_{and,or}_comparisons is defaulted to nullptr and I only adjusted the if-combine to pass down the outer block. I think other callers like tree-if-conv have the same issue but it's not straight-forward as to what to do there. 2022-04-05 Richard Biener PR tree-optimization/105142 * gimple-fold.h (maybe_fold_and_comparisons): Add defaulted basic-block parameter. (maybe_fold_or_comparisons): Likewise. * gimple-fold.cc (follow_outer_ssa_edges): New. (maybe_fold_comparisons_from_match_pd): Use follow_outer_ssa_edges when an outer condition basic-block is specified. (and_comparisons_1, and_var_with_comparison, and_var_with_comparison_1, or_comparisons_1, or_var_with_comparison, or_var_with_comparison_1): Receive and pass down the outer condition basic-block. * tree-ssa-ifcombine.cc (ifcombine_ifandif): Pass down the basic-block of the outer condition. * g++.dg/torture/pr105142.C: New testcase. --- gcc/gimple-fold.cc | 130 +++++++++++++++++++++----------- gcc/gimple-fold.h | 6 +- gcc/testsuite/g++.dg/torture/pr105142.C | 8 ++ gcc/tree-ssa-ifcombine.cc | 3 +- 4 files changed, 102 insertions(+), 45 deletions(-) create mode 100644 gcc/testsuite/g++.dg/torture/pr105142.C diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc index 270a9a2..97880a5 100644 --- a/gcc/gimple-fold.cc +++ b/gcc/gimple-fold.cc @@ -6537,22 +6537,27 @@ same_bool_result_p (const_tree op1, const_tree op2) static tree and_comparisons_1 (tree type, enum tree_code code1, tree op1a, tree op1b, - enum tree_code code2, tree op2a, tree op2b); + enum tree_code code2, tree op2a, tree op2b, basic_block); static tree and_var_with_comparison (tree type, tree var, bool invert, - enum tree_code code2, tree op2a, tree op2b); + enum tree_code code2, tree op2a, tree op2b, + basic_block); static tree and_var_with_comparison_1 (tree type, gimple *stmt, - enum tree_code code2, tree op2a, tree op2b); + enum tree_code code2, tree op2a, tree op2b, + basic_block); static tree or_comparisons_1 (tree, enum tree_code code1, tree op1a, tree op1b, - enum tree_code code2, tree op2a, tree op2b); + enum tree_code code2, tree op2a, tree op2b, + basic_block); static tree or_var_with_comparison (tree, tree var, bool invert, - enum tree_code code2, tree op2a, tree op2b); + enum tree_code code2, tree op2a, tree op2b, + basic_block); static tree or_var_with_comparison_1 (tree, gimple *stmt, - enum tree_code code2, tree op2a, tree op2b); + enum tree_code code2, tree op2a, tree op2b, + basic_block); /* Helper function for and_comparisons_1: try to simplify the AND of the ssa variable VAR with the comparison specified by (OP2A CODE2 OP2B). @@ -6561,7 +6566,8 @@ or_var_with_comparison_1 (tree, gimple *stmt, static tree and_var_with_comparison (tree type, tree var, bool invert, - enum tree_code code2, tree op2a, tree op2b) + enum tree_code code2, tree op2a, tree op2b, + basic_block outer_cond_bb) { tree t; gimple *stmt = SSA_NAME_DEF_STMT (var); @@ -6576,9 +6582,10 @@ and_var_with_comparison (tree type, tree var, bool invert, if (invert) t = or_var_with_comparison_1 (type, stmt, invert_tree_comparison (code2, false), - op2a, op2b); + op2a, op2b, outer_cond_bb); else - t = and_var_with_comparison_1 (type, stmt, code2, op2a, op2b); + t = and_var_with_comparison_1 (type, stmt, code2, op2a, op2b, + outer_cond_bb); return canonicalize_bool (t, invert); } @@ -6588,7 +6595,8 @@ and_var_with_comparison (tree type, tree var, bool invert, static tree and_var_with_comparison_1 (tree type, gimple *stmt, - enum tree_code code2, tree op2a, tree op2b) + enum tree_code code2, tree op2a, tree op2b, + basic_block outer_cond_bb) { tree var = gimple_assign_lhs (stmt); tree true_test_var = NULL_TREE; @@ -6623,7 +6631,7 @@ and_var_with_comparison_1 (tree type, gimple *stmt, gimple_assign_rhs2 (stmt), code2, op2a, - op2b); + op2b, outer_cond_bb); if (t) return t; } @@ -6655,12 +6663,12 @@ and_var_with_comparison_1 (tree type, gimple *stmt, return (is_and ? boolean_false_node : and_var_with_comparison (type, inner2, false, code2, op2a, - op2b)); + op2b, outer_cond_bb)); else if (inner2 == false_test_var) return (is_and ? boolean_false_node : and_var_with_comparison (type, inner1, false, code2, op2a, - op2b)); + op2b, outer_cond_bb)); /* Next, redistribute/reassociate the AND across the inner tests. Compute the first partial result, (inner1 AND (op2a code op2b)) */ @@ -6670,7 +6678,8 @@ and_var_with_comparison_1 (tree type, gimple *stmt, && (t = maybe_fold_and_comparisons (type, gimple_assign_rhs_code (s), gimple_assign_rhs1 (s), gimple_assign_rhs2 (s), - code2, op2a, op2b))) + code2, op2a, op2b, + outer_cond_bb))) { /* Handle the AND case, where we are reassociating: (inner1 AND inner2) AND (op2a code2 op2b) @@ -6702,7 +6711,8 @@ and_var_with_comparison_1 (tree type, gimple *stmt, && (t = maybe_fold_and_comparisons (type, gimple_assign_rhs_code (s), gimple_assign_rhs1 (s), gimple_assign_rhs2 (s), - code2, op2a, op2b))) + code2, op2a, op2b, + outer_cond_bb))) { /* Handle the AND case, where we are reassociating: (inner1 AND inner2) AND (op2a code2 op2b) @@ -6756,7 +6766,8 @@ and_var_with_comparison_1 (tree type, gimple *stmt, static tree and_comparisons_1 (tree type, enum tree_code code1, tree op1a, tree op1b, - enum tree_code code2, tree op2a, tree op2b) + enum tree_code code2, tree op2a, tree op2b, + basic_block outer_cond_bb) { tree truth_type = truth_type_for (TREE_TYPE (op1a)); @@ -6800,7 +6811,7 @@ and_comparisons_1 (tree type, enum tree_code code1, tree op1a, tree op1b, case GIMPLE_ASSIGN: /* Try to simplify by copy-propagating the definition. */ return and_var_with_comparison (type, op1a, invert, code2, op2a, - op2b); + op2b, outer_cond_bb); case GIMPLE_PHI: /* If every argument to the PHI produces the same result when @@ -6851,7 +6862,8 @@ and_comparisons_1 (tree type, enum tree_code code1, tree op1a, tree op1b, gimple_bb (stmt))) return NULL_TREE; temp = and_var_with_comparison (type, arg, invert, code2, - op2a, op2b); + op2a, op2b, + outer_cond_bb); if (!temp) return NULL_TREE; else if (!result) @@ -6872,6 +6884,25 @@ and_comparisons_1 (tree type, enum tree_code code1, tree op1a, tree op1b, return NULL_TREE; } +static basic_block fosa_bb; +static tree +follow_outer_ssa_edges (tree val) +{ + if (TREE_CODE (val) == SSA_NAME + && !SSA_NAME_IS_DEFAULT_DEF (val)) + { + basic_block def_bb = gimple_bb (SSA_NAME_DEF_STMT (val)); + if (!def_bb + || def_bb == fosa_bb + || (dom_info_available_p (CDI_DOMINATORS) + && (def_bb == fosa_bb + || dominated_by_p (CDI_DOMINATORS, fosa_bb, def_bb)))) + return val; + return NULL_TREE; + } + return val; +} + /* Helper function for maybe_fold_and_comparisons and maybe_fold_or_comparisons : try to simplify the AND/OR of the ssa variable VAR with the comparison specified by (OP2A CODE2 OP2B) from match.pd. Return NULL_EXPR if we can't @@ -6884,7 +6915,8 @@ maybe_fold_comparisons_from_match_pd (tree type, enum tree_code code, enum tree_code code1, tree op1a, tree op1b, enum tree_code code2, tree op2a, - tree op2b) + tree op2b, + basic_block outer_cond_bb) { /* Allocate gimple stmt1 on the stack. */ gassign *stmt1 @@ -6893,6 +6925,7 @@ maybe_fold_comparisons_from_match_pd (tree type, enum tree_code code, gimple_assign_set_rhs_code (stmt1, code1); gimple_assign_set_rhs1 (stmt1, op1a); gimple_assign_set_rhs2 (stmt1, op1b); + gimple_set_bb (stmt1, NULL); /* Allocate gimple stmt2 on the stack. */ gassign *stmt2 @@ -6901,6 +6934,7 @@ maybe_fold_comparisons_from_match_pd (tree type, enum tree_code code, gimple_assign_set_rhs_code (stmt2, code2); gimple_assign_set_rhs1 (stmt2, op2a); gimple_assign_set_rhs2 (stmt2, op2b); + gimple_set_bb (stmt2, NULL); /* Allocate SSA names(lhs1) on the stack. */ tree lhs1 = (tree)XALLOCA (tree_ssa_name); @@ -6922,7 +6956,9 @@ maybe_fold_comparisons_from_match_pd (tree type, enum tree_code code, gimple_match_op op (gimple_match_cond::UNCOND, code, type, gimple_assign_lhs (stmt1), gimple_assign_lhs (stmt2)); - if (op.resimplify (NULL, follow_all_ssa_edges)) + fosa_bb = outer_cond_bb; + if (op.resimplify (NULL, (!outer_cond_bb + ? follow_all_ssa_edges : follow_outer_ssa_edges))) { if (gimple_simplified_result_is_gimple_val (&op)) { @@ -6959,17 +6995,20 @@ maybe_fold_comparisons_from_match_pd (tree type, enum tree_code code, tree maybe_fold_and_comparisons (tree type, enum tree_code code1, tree op1a, tree op1b, - enum tree_code code2, tree op2a, tree op2b) + enum tree_code code2, tree op2a, tree op2b, + basic_block outer_cond_bb) { - if (tree t = and_comparisons_1 (type, code1, op1a, op1b, code2, op2a, op2b)) + if (tree t = and_comparisons_1 (type, code1, op1a, op1b, code2, op2a, op2b, + outer_cond_bb)) return t; - if (tree t = and_comparisons_1 (type, code2, op2a, op2b, code1, op1a, op1b)) + if (tree t = and_comparisons_1 (type, code2, op2a, op2b, code1, op1a, op1b, + outer_cond_bb)) return t; if (tree t = maybe_fold_comparisons_from_match_pd (type, BIT_AND_EXPR, code1, op1a, op1b, code2, op2a, - op2b)) + op2b, outer_cond_bb)) return t; return NULL_TREE; @@ -6982,7 +7021,8 @@ maybe_fold_and_comparisons (tree type, static tree or_var_with_comparison (tree type, tree var, bool invert, - enum tree_code code2, tree op2a, tree op2b) + enum tree_code code2, tree op2a, tree op2b, + basic_block outer_cond_bb) { tree t; gimple *stmt = SSA_NAME_DEF_STMT (var); @@ -6997,9 +7037,10 @@ or_var_with_comparison (tree type, tree var, bool invert, if (invert) t = and_var_with_comparison_1 (type, stmt, invert_tree_comparison (code2, false), - op2a, op2b); + op2a, op2b, outer_cond_bb); else - t = or_var_with_comparison_1 (type, stmt, code2, op2a, op2b); + t = or_var_with_comparison_1 (type, stmt, code2, op2a, op2b, + outer_cond_bb); return canonicalize_bool (t, invert); } @@ -7009,7 +7050,8 @@ or_var_with_comparison (tree type, tree var, bool invert, static tree or_var_with_comparison_1 (tree type, gimple *stmt, - enum tree_code code2, tree op2a, tree op2b) + enum tree_code code2, tree op2a, tree op2b, + basic_block outer_cond_bb) { tree var = gimple_assign_lhs (stmt); tree true_test_var = NULL_TREE; @@ -7042,9 +7084,7 @@ or_var_with_comparison_1 (tree type, gimple *stmt, tree t = or_comparisons_1 (type, innercode, gimple_assign_rhs1 (stmt), gimple_assign_rhs2 (stmt), - code2, - op2a, - op2b); + code2, op2a, op2b, outer_cond_bb); if (t) return t; } @@ -7076,12 +7116,12 @@ or_var_with_comparison_1 (tree type, gimple *stmt, return (is_or ? boolean_true_node : or_var_with_comparison (type, inner2, false, code2, op2a, - op2b)); + op2b, outer_cond_bb)); else if (inner2 == false_test_var) return (is_or ? boolean_true_node : or_var_with_comparison (type, inner1, false, code2, op2a, - op2b)); + op2b, outer_cond_bb)); /* Next, redistribute/reassociate the OR across the inner tests. Compute the first partial result, (inner1 OR (op2a code op2b)) */ @@ -7091,7 +7131,8 @@ or_var_with_comparison_1 (tree type, gimple *stmt, && (t = maybe_fold_or_comparisons (type, gimple_assign_rhs_code (s), gimple_assign_rhs1 (s), gimple_assign_rhs2 (s), - code2, op2a, op2b))) + code2, op2a, op2b, + outer_cond_bb))) { /* Handle the OR case, where we are reassociating: (inner1 OR inner2) OR (op2a code2 op2b) @@ -7123,7 +7164,8 @@ or_var_with_comparison_1 (tree type, gimple *stmt, && (t = maybe_fold_or_comparisons (type, gimple_assign_rhs_code (s), gimple_assign_rhs1 (s), gimple_assign_rhs2 (s), - code2, op2a, op2b))) + code2, op2a, op2b, + outer_cond_bb))) { /* Handle the OR case, where we are reassociating: (inner1 OR inner2) OR (op2a code2 op2b) @@ -7178,7 +7220,8 @@ or_var_with_comparison_1 (tree type, gimple *stmt, static tree or_comparisons_1 (tree type, enum tree_code code1, tree op1a, tree op1b, - enum tree_code code2, tree op2a, tree op2b) + enum tree_code code2, tree op2a, tree op2b, + basic_block outer_cond_bb) { tree truth_type = truth_type_for (TREE_TYPE (op1a)); @@ -7222,7 +7265,7 @@ or_comparisons_1 (tree type, enum tree_code code1, tree op1a, tree op1b, case GIMPLE_ASSIGN: /* Try to simplify by copy-propagating the definition. */ return or_var_with_comparison (type, op1a, invert, code2, op2a, - op2b); + op2b, outer_cond_bb); case GIMPLE_PHI: /* If every argument to the PHI produces the same result when @@ -7273,7 +7316,7 @@ or_comparisons_1 (tree type, enum tree_code code1, tree op1a, tree op1b, gimple_bb (stmt))) return NULL_TREE; temp = or_var_with_comparison (type, arg, invert, code2, - op2a, op2b); + op2a, op2b, outer_cond_bb); if (!temp) return NULL_TREE; else if (!result) @@ -7304,17 +7347,20 @@ or_comparisons_1 (tree type, enum tree_code code1, tree op1a, tree op1b, tree maybe_fold_or_comparisons (tree type, enum tree_code code1, tree op1a, tree op1b, - enum tree_code code2, tree op2a, tree op2b) + enum tree_code code2, tree op2a, tree op2b, + basic_block outer_cond_bb) { - if (tree t = or_comparisons_1 (type, code1, op1a, op1b, code2, op2a, op2b)) + if (tree t = or_comparisons_1 (type, code1, op1a, op1b, code2, op2a, op2b, + outer_cond_bb)) return t; - if (tree t = or_comparisons_1 (type, code2, op2a, op2b, code1, op1a, op1b)) + if (tree t = or_comparisons_1 (type, code2, op2a, op2b, code1, op1a, op1b, + outer_cond_bb)) return t; if (tree t = maybe_fold_comparisons_from_match_pd (type, BIT_IOR_EXPR, code1, op1a, op1b, code2, op2a, - op2b)) + op2b, outer_cond_bb)) return t; return NULL_TREE; diff --git a/gcc/gimple-fold.h b/gcc/gimple-fold.h index 82631a4..3a0ef54 100644 --- a/gcc/gimple-fold.h +++ b/gcc/gimple-fold.h @@ -33,9 +33,11 @@ extern bool fold_stmt (gimple_stmt_iterator *); extern bool fold_stmt (gimple_stmt_iterator *, tree (*) (tree)); extern bool fold_stmt_inplace (gimple_stmt_iterator *); extern tree maybe_fold_and_comparisons (tree, enum tree_code, tree, tree, - enum tree_code, tree, tree); + enum tree_code, tree, tree, + basic_block = nullptr); extern tree maybe_fold_or_comparisons (tree, enum tree_code, tree, tree, - enum tree_code, tree, tree); + enum tree_code, tree, tree, + basic_block = nullptr); extern bool clear_padding_type_may_have_padding_p (tree); extern void clear_type_padding_in_mask (tree, unsigned char *); extern bool optimize_atomic_compare_exchange_p (gimple *); diff --git a/gcc/testsuite/g++.dg/torture/pr105142.C b/gcc/testsuite/g++.dg/torture/pr105142.C new file mode 100644 index 0000000..b2d16ab --- /dev/null +++ b/gcc/testsuite/g++.dg/torture/pr105142.C @@ -0,0 +1,8 @@ +// { dg-do run { target lp64 } } + +long int c = 3623214276426624192L; +unsigned short b; +char a = 42; +const long &min(const long &x, const long &y) { return x < y ? x : y; } +__attribute__((noipa)) void test() { b = min(a, min(a, c) + 5713568809962283044L); } +int main() { test(); if (b != 42) __builtin_abort(); } diff --git a/gcc/tree-ssa-ifcombine.cc b/gcc/tree-ssa-ifcombine.cc index 3a4fee5..ce9bbeb 100644 --- a/gcc/tree-ssa-ifcombine.cc +++ b/gcc/tree-ssa-ifcombine.cc @@ -561,7 +561,8 @@ ifcombine_ifandif (basic_block inner_cond_bb, bool inner_inv, gimple_cond_rhs (inner_cond), outer_cond_code, gimple_cond_lhs (outer_cond), - gimple_cond_rhs (outer_cond)))) + gimple_cond_rhs (outer_cond), + gimple_bb (outer_cond)))) { tree t1, t2; gimple_stmt_iterator gsi; -- 2.7.4