| Message ID | ork41pe7hk.fsf@livre.localdomain |
|---|---|
| State | New |
| Headers | show |
On Mon, Apr 9, 2012 at 8:37 AM, Alexandre Oliva <aoliva@redhat.com> wrote: > On Jun 3, 2011, Alexandre Oliva <aoliva@redhat.com> wrote: > >> According to http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01082.html >> on Nov 20, 2009, Alexandre Oliva <aoliva@redhat.com> wrote: > >>> On Nov 19, 2009, Richard Guenther <richard.guenther@gmail.com> 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 “dying” 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) + /* 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); + that looks ... odd. But I see you want to conditionally compute value. +static inline bool +gimple_replace_lhs_wants_value (void) +{ + return MAY_HAVE_DEBUG_STMTS; +} but should this not depend on the old stmt / lhs? For example do we really want to do this for artificial variables? I suppose not. Thanks, Richard. > > > -- > 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 >
Sorry I dropped the ball on this one. Context is here: http://gcc.gnu.org/ml/gcc-patches/2012-04/msg00416.html On Apr 12, 2012, Richard Guenther <richard.guenther@gmail.com> wrote: > + /* 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); > + > that looks ... odd. But I see you want to conditionally compute value. > +static inline bool > +gimple_replace_lhs_wants_value (void) > +{ > + return MAY_HAVE_DEBUG_STMTS; > +} > but should this not depend on the old stmt / lhs? For example do we really > want to do this for artificial variables? I suppose not. I think we do. We want to preserve the value of the expression when it is referenced in other debug expressions. For these other uses, it doesn't matter whether this value happened to be computed and stored in a temporary, an artificial variable or a user-defined variable.
On Apr 12, 2012, Richard Guenther <richard.guenther@gmail.com> wrote: > + /* 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) { > that looks ... odd. Indeed, it does. Does this look any better? bool save_value = MAY_HAVE_DEBUG_STMTS; /* If the condition above, in which this function uses VALUE change, adjust gimple_replace_lhs_wants_value() to match. The assert below helps enforce this. */ gcc_checking_assert (gimple_replace_lhs_wants_value () == save_value); if (save_value) {
for gcc/ChangeLog from Alexandre Oliva <aoliva@redhat.com> 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);