From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29269 invoked by alias); 9 Apr 2012 06:37:33 -0000 Received: (qmail 28998 invoked by uid 22791); 9 Apr 2012 06:37:30 -0000 X-SWARE-Spam-Status: No, hits=-5.4 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_TM,T_RP_MATCHES_RCVD,T_TVD_MIME_NO_HEADERS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 09 Apr 2012 06:37:15 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q396bFVZ031174 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 9 Apr 2012 02:37:15 -0400 Received: from freie.oliva.athome.lsd.ic.unicamp.br (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q396bDcB026422 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 9 Apr 2012 02:37:14 -0400 Received: from livre.localdomain (livre.oliva.athome.lsd.ic.unicamp.br [172.31.160.2]) by freie.oliva.athome.lsd.ic.unicamp.br (8.14.5/8.14.5) with ESMTP id q396bCoU009864; Mon, 9 Apr 2012 03:37:13 -0300 Received: from livre.localdomain (aoliva@localhost.localdomain [127.0.0.1]) by livre.localdomain (8.14.3/8.14.3/Debian-5+lenny1) with ESMTP id q396bC5Y002263; Mon, 9 Apr 2012 03:37:12 -0300 Received: (from aoliva@localhost) by livre.localdomain (8.14.3/8.14.3/Submit) id q396bB78002261; Mon, 9 Apr 2012 03:37:11 -0300 From: Alexandre Oliva To: Richard Guenther Cc: Jakub Jelinek , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Fix ICE in expand_cse_reciprocals (PR tree-optimization/42078) References: <20091118173010.GL3047@sunsite.ms.mff.cuni.cz> <84fc9c000911190615v65576f2ahd76a0f28b3f725ae@mail.gmail.com> Date: Mon, 09 Apr 2012 06:37:00 -0000 In-Reply-To: (Alexandre Oliva's message of "Fri, 03 Jun 2011 11:28:53 -0300") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2012-04/txt/msg00416.txt.bz2 --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Content-length: 1426 On Jun 3, 2011, Alexandre Oliva wrote: > According to http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01082.html > on Nov 20, 2009, Alexandre Oliva wrote: >> On Nov 19, 2009, Richard Guenther wrote: >>> In fact this exchanging of the LHS (or rather invalidating of the >>> SSA name value) should be a helper function that knows >>> the implementation details and avoids going through releasing >>> and allocating the name. >> Okie dokie, here's a sequence of patches that implements helper >> functions for this sort of stuff. >> The first patch introduces machinery to propagate =E2=80=9Cdying=E2=80= =9D DEFs into >> debug stmts, while replacing them with other SSA_NAMEs. > This is already in. >> The second extends it so as to enable the old LHS to be redefined >> e.g. in terms of the new LHS. IIRC this may be useful in some other >> transformations that, in the process of introducing VTA, I changed from >> modifying stmts in place to inserting new stmts and removing others. I >> haven't looked for them yet. >> The third uses this new feature for the case at hand, while avoiding >> computing the reciprocal expression if we know it won't be used. > Updated versions of these follow. Regstrapped on x86_64-linux-gnu and > i686-linux-gnu. Ok to install? I was asked to submit these again in stage1, so... Ping? (updated and re-tested) --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=vta-debug-temps-explicit-values-pr42078.patch Content-length: 9599 for gcc/ChangeLog from Alexandre Oliva PR tree-optimization/42078 * gimple.h (gimple_replace_lhs_wants_value): New. (gimple_replace_lhs): Add value. * gimple.c (gimple_replace_lhs): Likewise. Pass it on. * tree-flow.h (insert_debug_temp_for_var_def): Add value. * tree-ssa.c (insert_debug_temp_for_var_def): Likewise. Use it. (insert_debug_temps_for_defs): Pass NULL value. * tree-ssanames.c (release_ssa_name): Likewise. * tree-ssa-math-opts.c (execute_cse_reciprocals): Likewise. * gcc/tree-ssa-reassoc.c (repropagate_negates): Likewise. Index: gcc/gimple.h =================================================================== --- gcc/gimple.h.orig 2012-04-07 20:24:41.796529955 -0300 +++ gcc/gimple.h 2012-04-08 02:06:44.417457641 -0300 @@ -943,7 +943,7 @@ void gimple_assign_set_rhs_with_ops_1 (g tree, tree, tree); tree gimple_get_lhs (const_gimple); void gimple_set_lhs (gimple, tree); -void gimple_replace_lhs (gimple, tree); +void gimple_replace_lhs (gimple, tree, tree); gimple gimple_copy (gimple); void gimple_set_modified (gimple, bool); void gimple_cond_get_ops_from_tree (tree, enum tree_code *, tree *, tree *); @@ -2521,6 +2521,14 @@ gimple_has_lhs (gimple stmt) && gimple_call_lhs (stmt) != NULL_TREE)); } +/* Return true if it might be useful to pass a VALUE to + gimple_replace_lhs (). */ +static inline bool +gimple_replace_lhs_wants_value (void) +{ + return MAY_HAVE_DEBUG_STMTS; +} + /* Return the code of the predicate computed by conditional statement GS. */ Index: gcc/gimple.c =================================================================== --- gcc/gimple.c.orig 2012-04-07 20:24:41.796529955 -0300 +++ gcc/gimple.c 2012-04-08 02:06:44.606455321 -0300 @@ -2239,10 +2239,10 @@ gimple_set_lhs (gimple stmt, tree lhs) expression with a different value. This will update any annotations (say debug bind stmts) referring - to the original LHS, so that they use the RHS instead. This is - done even if NLHS and LHS are the same, for it is understood that - the RHS will be modified afterwards, and NLHS will not be assigned - an equivalent value. + to the original LHS, so that they use VALUE or the RHS instead. + This is done even if NLHS and LHS are the same, for it is + understood that the RHS will be modified afterwards, and NLHS will + not be assigned an equivalent value. Adjusting any non-annotation uses of the LHS, if needed, is a responsibility of the caller. @@ -2254,15 +2254,20 @@ gimple_set_lhs (gimple stmt, tree lhs) copying and removing. */ void -gimple_replace_lhs (gimple stmt, tree nlhs) +gimple_replace_lhs (gimple stmt, tree nlhs, tree value) { + /* If the conditions in which this function uses VALUE change, + adjust gimple_replace_lhs_wants_value(). */ + gcc_assert (gimple_replace_lhs_wants_value () + == MAY_HAVE_DEBUG_STMTS); + if (MAY_HAVE_DEBUG_STMTS) { tree lhs = gimple_get_lhs (stmt); gcc_assert (SSA_NAME_DEF_STMT (lhs) == stmt); - insert_debug_temp_for_var_def (NULL, lhs); + insert_debug_temp_for_var_def (NULL, lhs, value); } gimple_set_lhs (stmt, nlhs); Index: gcc/tree-flow.h =================================================================== --- gcc/tree-flow.h.orig 2012-04-07 20:24:45.350481393 -0300 +++ gcc/tree-flow.h 2012-04-08 02:06:44.658454684 -0300 @@ -563,7 +563,7 @@ typedef bool (*walk_use_def_chains_fn) ( extern void walk_use_def_chains (tree, walk_use_def_chains_fn, void *, bool); void insert_debug_temps_for_defs (gimple_stmt_iterator *); -void insert_debug_temp_for_var_def (gimple_stmt_iterator *, tree); +void insert_debug_temp_for_var_def (gimple_stmt_iterator *, tree, tree); void reset_debug_uses (gimple); void release_defs_bitset (bitmap toremove); Index: gcc/tree-ssa.c =================================================================== --- gcc/tree-ssa.c.orig 2012-02-25 09:45:32.788781511 -0200 +++ gcc/tree-ssa.c 2012-04-08 02:06:44.941451213 -0300 @@ -300,17 +300,25 @@ find_released_ssa_name (tree *tp, int *w /* Insert a DEBUG BIND stmt before the DEF of VAR if VAR is referenced by other DEBUG stmts, and replace uses of the DEF with the - newly-created debug temp. */ + newly-created debug temp, or with the DEF's value, if the + substitution is valid. The value is taken from VALUE, if given, or + from the DEF stmt, taken from GSI, if given, or from the DEF of + VAR. If VALUE and VAR are the same, no DEBUG BIND stmt will be + created, and all uses of VAR will be reset. If a DEBUG BIND stmt + has to be created, it will be inserted before or after GSI or the + DEF, depending */ void -insert_debug_temp_for_var_def (gimple_stmt_iterator *gsi, tree var) +insert_debug_temp_for_var_def (gimple_stmt_iterator *gsi, tree var, tree value) { imm_use_iterator imm_iter; use_operand_p use_p; gimple stmt; gimple def_stmt = NULL; int usecount = 0; - tree value = NULL; + bool given_value; + gimple def_temp = NULL; + gimple_stmt_iterator ngsi; if (!MAY_HAVE_DEBUG_STMTS) return; @@ -350,10 +358,13 @@ insert_debug_temp_for_var_def (gimple_st else def_stmt = SSA_NAME_DEF_STMT (var); + given_value = false; + if (value) + given_value = true; /* If we didn't get an insertion point, and the stmt has already been removed, we won't be able to insert the debug bind stmt, so we'll have to drop debug information. */ - if (gimple_code (def_stmt) == GIMPLE_PHI) + else if (gimple_code (def_stmt) == GIMPLE_PHI) { value = degenerate_phi_result (def_stmt); if (value && walk_tree (&value, find_released_ssa_name, NULL, NULL)) @@ -408,6 +419,9 @@ insert_debug_temp_for_var_def (gimple_st value = gimple_assign_rhs_to_tree (def_stmt); } + if (value == var) + value = NULL; + if (value) { /* If there's a single use of VAR, and VAR is the entire debug @@ -425,7 +439,7 @@ insert_debug_temp_for_var_def (gimple_st at the expense of duplication of expressions. */ if (CONSTANT_CLASS_P (value) - || gimple_code (def_stmt) == GIMPLE_PHI + || (gimple_code (def_stmt) == GIMPLE_PHI && !given_value) || (usecount == 1 && (!gimple_assign_single_p (def_stmt) || is_gimple_min_invariant (value))) @@ -433,7 +447,6 @@ insert_debug_temp_for_var_def (gimple_st value = unshare_expr (value); else { - gimple def_temp; tree vexpr = make_node (DEBUG_EXPR_DECL); def_temp = gimple_build_debug_bind (vexpr, @@ -447,14 +460,27 @@ insert_debug_temp_for_var_def (gimple_st else DECL_MODE (vexpr) = TYPE_MODE (TREE_TYPE (value)); - if (gsi) - gsi_insert_before (gsi, def_temp, GSI_SAME_STMT); - else + if (gimple_code (def_stmt) == GIMPLE_PHI) + { + basic_block bb; + + if (gsi) + bb = gsi_bb (*gsi); + else + bb = gimple_bb (def_stmt); + + ngsi = gsi_after_labels (bb); + gsi = &ngsi; + } + else if (!gsi) { - gimple_stmt_iterator ngsi = gsi_for_stmt (def_stmt); - gsi_insert_before (&ngsi, def_temp, GSI_SAME_STMT); + ngsi = gsi_for_stmt (def_stmt); + gsi = &ngsi; } + if (!given_value) + gsi_insert_before (gsi, def_temp, GSI_SAME_STMT); + value = vexpr; } } @@ -486,6 +512,11 @@ insert_debug_temp_for_var_def (gimple_st update_stmt (stmt); } + + /* Insert the definition last, so that we don't modify it in case + VALUE references VAR. Assume the caller intends it to be so. */ + if (def_temp && given_value) + gsi_insert_after (gsi, def_temp, GSI_SAME_STMT); } @@ -512,7 +543,7 @@ insert_debug_temps_for_defs (gimple_stmt if (TREE_CODE (var) != SSA_NAME) continue; - insert_debug_temp_for_var_def (gsi, var); + insert_debug_temp_for_var_def (gsi, var, NULL); } } Index: gcc/tree-ssanames.c =================================================================== --- gcc/tree-ssanames.c.orig 2012-04-07 20:24:45.383480943 -0300 +++ gcc/tree-ssanames.c 2012-04-08 02:06:45.215447852 -0300 @@ -203,7 +203,7 @@ release_ssa_name (tree var) use_operand_p imm = &(SSA_NAME_IMM_USE_NODE (var)); if (MAY_HAVE_DEBUG_STMTS) - insert_debug_temp_for_var_def (NULL, var); + insert_debug_temp_for_var_def (NULL, var, NULL); #ifdef ENABLE_CHECKING verify_imm_links (stderr, var); Index: gcc/tree-ssa-math-opts.c =================================================================== --- gcc/tree-ssa-math-opts.c.orig 2012-04-07 20:24:45.354481340 -0300 +++ gcc/tree-ssa-math-opts.c 2012-04-08 02:06:45.383445792 -0300 @@ -604,7 +604,7 @@ execute_cse_reciprocals (void) if (fail) continue; - gimple_replace_lhs (stmt1, arg1); + gimple_replace_lhs (stmt1, arg1, NULL); gimple_call_set_fndecl (stmt1, fndecl); update_stmt (stmt1); reciprocal_stats.rfuncs_inserted++; Index: gcc/tree-ssa-reassoc.c =================================================================== --- gcc/tree-ssa-reassoc.c.orig 2011-10-04 02:32:44.000000000 -0300 +++ gcc/tree-ssa-reassoc.c 2012-04-08 02:06:45.923439168 -0300 @@ -2719,7 +2719,7 @@ repropagate_negates (void) tree a = gimple_assign_rhs1 (feed); tree rhs2 = gimple_assign_rhs2 (user); gimple_stmt_iterator gsi = gsi_for_stmt (feed), gsi2; - gimple_replace_lhs (feed, negate); + gimple_replace_lhs (feed, negate, NULL); gimple_assign_set_rhs_with_ops (&gsi, PLUS_EXPR, a, rhs2); update_stmt (gsi_stmt (gsi)); gsi2 = gsi_for_stmt (user); --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=vta-cse-reciprocals-preserve-pr42078.patch Content-length: 1512 for gcc/ChangeLog from Alexandre Oliva PR tree-optimization/42078 * tree-ssa-math-opts.c (execute_cse_reciprocals): Compute reciprocal value for debug stmts. Index: gcc/tree-ssa-math-opts.c =================================================================== --- gcc/tree-ssa-math-opts.c.orig 2012-04-08 02:06:45.383445792 -0300 +++ gcc/tree-ssa-math-opts.c 2012-04-08 02:09:53.000000000 -0300 @@ -575,6 +575,7 @@ execute_cse_reciprocals (void) bool md_code, fail; imm_use_iterator ui; use_operand_p use_p; + tree value; code = DECL_FUNCTION_CODE (fndecl); md_code = DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_MD; @@ -604,14 +605,26 @@ execute_cse_reciprocals (void) if (fail) continue; - gimple_replace_lhs (stmt1, arg1, NULL); + if (gimple_replace_lhs_wants_value ()) + { + tree t = TREE_TYPE (arg1); + + value = build2 (RDIV_EXPR, t, build_one_cst (t), arg1); + } + else + value = NULL; + + gimple_replace_lhs (stmt1, arg1, value); gimple_call_set_fndecl (stmt1, fndecl); update_stmt (stmt1); reciprocal_stats.rfuncs_inserted++; FOR_EACH_IMM_USE_STMT (stmt, ui, arg1) { - gimple_stmt_iterator gsi = gsi_for_stmt (stmt); + gimple_stmt_iterator gsi; + if (is_gimple_debug (stmt)) + continue; + gsi = gsi_for_stmt (stmt); gimple_assign_set_rhs_code (stmt, MULT_EXPR); fold_stmt_inplace (&gsi); update_stmt (stmt); --=-=-= Content-length: 258 -- Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist Red Hat Brazil Compiler Engineer --=-=-=--