| Message ID | orpqsz1qm3.fsf@livre.localdomain |
|---|---|
| State | New |
| Headers | show |
On Sat, Dec 18, 2010 at 8:34 AM, Alexandre Oliva <aoliva@redhat.com> wrote: > The tree vectorizer sometimes creates DEFs and removes them before the > end of the pass. By the time of the removal, the SSA name is still > marked as needing updates, so we skip the code that might introduce a > debug temp before the DEF. > > By the time we actually release the DEF, calling that code again, the > DEF is no longer in the stmt seq, so we can't tell where to insert the > debug temp. > > We could just drop the debug info on the floor, but if the value is > unchanging, we can still propagate it to debug stmts that use that SSA > name. This is what this patch does, also avoiding the ICE we got trying > to figure out where to insert the debug temp. > > Regstrapped on x86_64-linux-gnu. Ok to install? The statements are artificially created by the vectorizer, we should not create extra debug info for them. They are also the only statements that are supposed to be removed by the very simple DCE - but we cannot identify them reliably. Is this patch not adding another hack above the DCE hack I installed? 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 > >
On Dec 18, 2010, Richard Guenther <richard.guenther@gmail.com> wrote: > The statements are artificially created by the vectorizer, Not really. The loop contains these statements and debug stmts. SCCP replaces the end-of-loop PHI nodes with final value computations, making the in-loop DEF dead, but not removing it. The vectorizer actually vectorizes the loop. While doing so, it duplicates the original loop into pre- and post- loops, for proper loop alignment. Because of the renaming in the post-vectorized loop, we examine the copied DEF in there and realize it is dead, so we remove it. If we don't propagate the DEF into the copied debug stmts, we'll make the variable incorrect within the post-loop. > They are also the only statements that are supposed to be removed by > the very simple DCE - but we cannot identify them reliably. Is this > patch not adding another hack above the DCE hack I installed? Dunno, really. Sure we run this very simple DCE right after SCCP, so as to remove the dead in-loop DEF then? It might obviate this change, but it's not unreasonable to put it in anyway, I think, especially after reinstate a more general condition to enable the copying. The is_gimple_min_invariant() test was a last-minute thought that was supposed to be extended so as to handle the case at hand but not other expressions that can't be moved about, but that I forgot about before testing and posting the patch. Oops ;-)
for gcc/ChangeLog from Alexandre Oliva <aoliva@redhat.com> PR debug/46931 * tree-ssa.c (insert_debug_temp_for_var_def): Handle removed assignments. Index: gcc/tree-ssa.c =================================================================== --- gcc/tree-ssa.c.orig 2010-12-17 01:07:57.818499387 -0200 +++ gcc/tree-ssa.c 2010-12-17 04:01:43.577493644 -0200 @@ -305,6 +305,8 @@ insert_debug_temp_for_var_def (gimple_st gimple def_stmt = NULL; int usecount = 0; tree value = NULL; + tree value_unshare = NULL; + tree temp_value = NULL; if (!MAY_HAVE_DEBUG_STMTS) return; @@ -421,6 +423,12 @@ insert_debug_temp_for_var_def (gimple_st || is_gimple_min_invariant (value))) || is_gimple_reg (value)) value = unshare_expr (value); + else if (!gsi && !gimple_bb (def_stmt)) + { + if (is_gimple_min_invariant (value)) + value_unshare = value; + value = NULL; + } else { gimple def_temp; @@ -454,13 +462,48 @@ insert_debug_temp_for_var_def (gimple_st if (!gimple_debug_bind_p (stmt)) continue; + /* If we have a value that needs unsharing, unshare it. Then, + if the debug stmt binds to VAR, we can replace it, otherwise + we'll create a debug temp, bind it to the unshared value + right before STMT, and replace uses of VAR with the debug + temp. We reuse the same temp for multiple uses, but we don't + attempt to avoid emitting debug temps that would be dominated + by identical debug bind stmts. */ + if (value_unshare) + { + value = unshare_expr (value_unshare); + if (gimple_debug_bind_get_value (stmt) != var) + { + gimple def_temp; + gimple_stmt_iterator ngsi = gsi_for_stmt (stmt); + + if (!temp_value) + { + temp_value = make_node (DEBUG_EXPR_DECL); + + DECL_ARTIFICIAL (temp_value) = 1; + TREE_TYPE (temp_value) = TREE_TYPE (value); + if (DECL_P (value)) + DECL_MODE (temp_value) = DECL_MODE (value); + else + DECL_MODE (temp_value) = TYPE_MODE (TREE_TYPE (value)); + } + + def_temp = gimple_build_debug_bind (temp_value, value, + def_stmt); + + gsi_insert_before (&ngsi, def_temp, GSI_SAME_STMT); + + value = temp_value; + } + } + if (value) FOR_EACH_IMM_USE_ON_STMT (use_p, imm_iter) - /* unshare_expr is not needed here. vexpr is either a + /* unshare_expr is not needed here. value is either a SINGLE_RHS, that can be safely shared, some other RHS that was unshared when we found it had a single debug - use, or a DEBUG_EXPR_DECL, that can be safely - shared. */ + use, or a DEBUG_EXPR_DECL, that can be safely shared. */ SET_USE (use_p, value); else gimple_debug_bind_reset_value (stmt);