From patchwork Wed Jul 20 12:16:43 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 105667 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id D81F9B6F68 for ; Wed, 20 Jul 2011 22:17:07 +1000 (EST) Received: (qmail 15662 invoked by alias); 20 Jul 2011 12:17:03 -0000 Received: (qmail 15651 invoked by uid 22791); 20 Jul 2011 12:17:01 -0000 X-SWARE-Spam-Status: No, hits=-3.8 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, TW_CF, TW_TM X-Spam-Check-By: sourceware.org Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 20 Jul 2011 12:16:45 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.221.2]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id 976FE8F903 for ; Wed, 20 Jul 2011 14:16:43 +0200 (CEST) Date: Wed, 20 Jul 2011 14:16:43 +0200 (CEST) From: Richard Guenther To: gcc-patches@gcc.gnu.org Subject: [PATCH] Fix condition propagation in forwprop, fixup locations in phiopt Message-ID: User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) MIME-Version: 1.0 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org This fixes one of the fallouts when boolifying comparisons. Namely that we do not manage to propagate multiple times into a condition statement. That's because we fail to remove unused defs and thus do not maintain a single-use chain for the second propagation. Fixed by removing dead defs after simplifying a condition. Phi-opt also forgot to set proper locations on the comparison it emits as separate statement which lead to even more confusing locations printed in warnings. Also fixed (with that fix only we report for the xfailed location which would also be ok). Bootstrap and regtest running on x86_64-unknown-linux-gnu, I'll commit this if that succeeds. Richard. 2011-07-20 Richard Guenther * tree-ssa-forwprop.c (forward_propagate_into_comparison): Remove dead defining stmts. (forward_propagate_into_gimple_cond): Likewise. (forward_propagate_into_cond): Simplify. (ssa_forward_propagate_and_combine): Handle changed cfg from forward_propagate_into_comparison. * tree-ssa-phiopt.c (conditional_replacement): Use proper locations for newly built statements. Index: gcc/tree-ssa-phiopt.c =================================================================== *** gcc/tree-ssa-phiopt.c (revision 176497) --- gcc/tree-ssa-phiopt.c (working copy) *************** conditional_replacement (basic_block con *** 544,551 **** /* To handle special cases like floating point comparison, it is easier and less error-prone to build a tree and gimplify it on the fly though it is less efficient. */ ! cond = fold_build2 (gimple_cond_code (stmt), boolean_type_node, ! gimple_cond_lhs (stmt), gimple_cond_rhs (stmt)); /* We need to know which is the true edge and which is the false edge so that we know when to invert the condition below. */ --- 544,552 ---- /* To handle special cases like floating point comparison, it is easier and less error-prone to build a tree and gimplify it on the fly though it is less efficient. */ ! cond = fold_build2_loc (gimple_location (stmt), ! gimple_cond_code (stmt), boolean_type_node, ! gimple_cond_lhs (stmt), gimple_cond_rhs (stmt)); /* We need to know which is the true edge and which is the false edge so that we know when to invert the condition below. */ *************** conditional_replacement (basic_block con *** 554,560 **** || (e0 == false_edge && integer_onep (arg0)) || (e1 == true_edge && integer_zerop (arg1)) || (e1 == false_edge && integer_onep (arg1))) ! cond = fold_build1 (TRUTH_NOT_EXPR, TREE_TYPE (cond), cond); /* Insert our new statements at the end of conditional block before the COND_STMT. */ --- 555,562 ---- || (e0 == false_edge && integer_onep (arg0)) || (e1 == true_edge && integer_zerop (arg1)) || (e1 == false_edge && integer_onep (arg1))) ! cond = fold_build1_loc (gimple_location (stmt), ! TRUTH_NOT_EXPR, TREE_TYPE (cond), cond); /* Insert our new statements at the end of conditional block before the COND_STMT. */ Index: gcc/tree-ssa-forwprop.c =================================================================== --- gcc/tree-ssa-forwprop.c.orig 2011-07-20 14:04:03.000000000 +0200 +++ gcc/tree-ssa-forwprop.c 2011-07-20 14:02:28.000000000 +0200 @@ -437,29 +437,36 @@ forward_propagate_into_comparison_1 (loc /* Propagate from the ssa name definition statements of the assignment from a comparison at *GSI into the conditional if that simplifies it. - Returns true if the stmt was modified, false if not. */ + Returns 1 if the stmt was modified and 2 if the CFG needs cleanup, + otherwise returns 0. */ -static bool +static int forward_propagate_into_comparison (gimple_stmt_iterator *gsi) { gimple stmt = gsi_stmt (*gsi); tree tmp; + bool cfg_changed = false; + tree rhs1 = gimple_assign_rhs1 (stmt); + tree rhs2 = gimple_assign_rhs2 (stmt); /* Combine the comparison with defining statements. */ tmp = forward_propagate_into_comparison_1 (gimple_location (stmt), gimple_assign_rhs_code (stmt), TREE_TYPE (gimple_assign_lhs (stmt)), - gimple_assign_rhs1 (stmt), - gimple_assign_rhs2 (stmt)); + rhs1, rhs2); if (tmp) { gimple_assign_set_rhs_from_tree (gsi, tmp); update_stmt (stmt); - return true; + if (TREE_CODE (rhs1) == SSA_NAME) + cfg_changed |= remove_prop_source_from_use (rhs1); + if (TREE_CODE (rhs2) == SSA_NAME) + cfg_changed |= remove_prop_source_from_use (rhs2); + return cfg_changed ? 2 : 1; } - return false; + return 0; } /* Propagate from the ssa name definition statements of COND_EXPR @@ -472,10 +479,12 @@ forward_propagate_into_comparison (gimpl static int forward_propagate_into_gimple_cond (gimple stmt) { - int did_something = 0; location_t loc = gimple_location (stmt); tree tmp; enum tree_code code = gimple_cond_code (stmt); + bool cfg_changed = false; + tree rhs1 = gimple_cond_lhs (stmt); + tree rhs2 = gimple_cond_rhs (stmt); /* We can do tree combining on SSA_NAME and comparison expressions. */ if (TREE_CODE_CLASS (gimple_cond_code (stmt)) != tcc_comparison) @@ -483,18 +492,13 @@ forward_propagate_into_gimple_cond (gimp tmp = forward_propagate_into_comparison_1 (loc, code, boolean_type_node, - gimple_cond_lhs (stmt), - gimple_cond_rhs (stmt)); + rhs1, rhs2); if (tmp) { if (dump_file && tmp) { - tree cond = build2 (gimple_cond_code (stmt), - boolean_type_node, - gimple_cond_lhs (stmt), - gimple_cond_rhs (stmt)); fprintf (dump_file, " Replaced '"); - print_generic_expr (dump_file, cond, 0); + print_gimple_expr (dump_file, stmt, 0, 0); fprintf (dump_file, "' with '"); print_generic_expr (dump_file, tmp, 0); fprintf (dump_file, "'\n"); @@ -503,14 +507,14 @@ forward_propagate_into_gimple_cond (gimp gimple_cond_set_condition_from_tree (stmt, unshare_expr (tmp)); update_stmt (stmt); - /* Remove defining statements. */ - if (is_gimple_min_invariant (tmp)) - did_something = 2; - else if (did_something == 0) - did_something = 1; + if (TREE_CODE (rhs1) == SSA_NAME) + cfg_changed |= remove_prop_source_from_use (rhs1); + if (TREE_CODE (rhs2) == SSA_NAME) + cfg_changed |= remove_prop_source_from_use (rhs2); + return (cfg_changed || is_gimple_min_invariant (tmp)) ? 2 : 1; } - return did_something; + return 0; } @@ -526,7 +530,6 @@ forward_propagate_into_cond (gimple_stmt { gimple stmt = gsi_stmt (*gsi_p); location_t loc = gimple_location (stmt); - int did_something = 0; tree tmp = NULL_TREE; tree cond = gimple_assign_rhs1 (stmt); @@ -541,7 +544,7 @@ forward_propagate_into_cond (gimple_stmt tree name = cond, rhs0; gimple def_stmt = get_prop_source_stmt (name, true, NULL); if (!def_stmt || !can_propagate_from (def_stmt)) - return did_something; + return 0; rhs0 = gimple_assign_rhs1 (def_stmt); tmp = combine_cond_expr_cond (loc, NE_EXPR, boolean_type_node, rhs0, @@ -564,14 +567,10 @@ forward_propagate_into_cond (gimple_stmt stmt = gsi_stmt (*gsi_p); update_stmt (stmt); - /* Remove defining statements. */ - if (is_gimple_min_invariant (tmp)) - did_something = 2; - else if (did_something == 0) - did_something = 1; + return is_gimple_min_invariant (tmp) ? 2 : 1; } - return did_something; + return 0; } /* We've just substituted an ADDR_EXPR into stmt. Update all the @@ -2433,11 +2432,15 @@ ssa_forward_propagate_and_combine (void) else if (TREE_CODE_CLASS (code) == tcc_comparison) { bool no_warning = gimple_no_warning_p (stmt); + int did_something; fold_defer_overflow_warnings (); - changed = forward_propagate_into_comparison (&gsi); + did_something = forward_propagate_into_comparison (&gsi); + if (did_something == 2) + cfg_changed = true; fold_undefer_overflow_warnings (!no_warning && changed, stmt, WARN_STRICT_OVERFLOW_CONDITIONAL); + changed = did_something != 0; } else if (code == BIT_AND_EXPR || code == BIT_IOR_EXPR