This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Fix PR43077, debug regression with SSA expand
Hi,
On Fri, 19 Feb 2010, Michael Matz wrote:
> Due to some discussion I retract this one for now. It would do the wrong
> thing for such situation:
>
> a_1 = b_2 + c_3 // assume a_1 is TERed
> ... use(a_1) // only real use of a_1
> b_3 = ...
> #DEBUG something => f(a_1)
>
> When forwarding the RHS "b_2 + c_3" into the debug use we might have
> clobbered b_2 already, namely when b_3 and b_2 can be coalesced. I'm now
> planning to use debug temps for this situation.
Like so. Regstrapped on x86_64-linux, no regressions (and since then only
added the comments). Okay for trunk?
Ciao,
Michael.
--
PR debug/43077
* cfgexpand (expand_debug_expr): Expand TERed ssa names in place.
(expand_gimple_basic_block): Generate and use debug temps if there
are debug uses left after the last real use of TERed ssa names.
Unlink debug immediate uses when they are expanded.
testsuite/
2010-02-19 Jakub Jelinek <jakub@redhat.com>
PR debug/43077
* gcc.dg/guality/pr43077-1.c: New test.
Index: cfgexpand.c
===================================================================
*** cfgexpand.c (revision 156895)
--- cfgexpand.c (working copy)
*************** expand_debug_expr (tree exp)
*** 2338,2344 ****
if (inner_mode == VOIDmode)
{
! inner_mode = TYPE_MODE (TREE_TYPE (TREE_OPERAND (exp, 0)));
if (mode == inner_mode)
return op0;
}
--- 2338,2347 ----
if (inner_mode == VOIDmode)
{
! if (TREE_CODE (exp) == SSA_NAME)
! inner_mode = TYPE_MODE (TREE_TYPE (exp));
! else
! inner_mode = TYPE_MODE (TREE_TYPE (TREE_OPERAND (exp, 0)));
if (mode == inner_mode)
return op0;
}
*************** expand_debug_expr (tree exp)
*** 2354,2359 ****
--- 2357,2363 ----
}
else if (FLOAT_MODE_P (mode))
{
+ gcc_assert (TREE_CODE (exp) != SSA_NAME);
if (TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (exp, 0))))
op0 = simplify_gen_unary (UNSIGNED_FLOAT, mode, op0, inner_mode);
else
*************** expand_debug_expr (tree exp)
*** 2886,2899 ****
case SSA_NAME:
{
! int part = var_to_partition (SA.map, exp);
! if (part == NO_PARTITION)
! return NULL;
! gcc_assert (part >= 0 && (unsigned)part < SA.map->num_partitions);
! op0 = SA.partition_to_pseudo[part];
goto adjust_mode;
}
--- 2890,2913 ----
case SSA_NAME:
{
! gimple g = get_gimple_for_ssa_name (exp);
! if (g)
! {
! op0 = expand_debug_expr (gimple_assign_rhs_to_tree (g));
! if (!op0)
! return NULL;
! }
! else
! {
! int part = var_to_partition (SA.map, exp);
! if (part == NO_PARTITION)
! return NULL;
! gcc_assert (part >= 0 && (unsigned)part < SA.map->num_partitions);
! op0 = SA.partition_to_pseudo[part];
! }
goto adjust_mode;
}
*************** expand_gimple_basic_block (basic_block b
*** 3050,3055 ****
--- 3064,3168 ----
basic_block new_bb;
stmt = gsi_stmt (gsi);
+
+ /* If this statement is a non-debug one, and we generate debug
+ insns, then this one might be the last real use of a TERed
+ SSA_NAME, but where there are still some debug uses further
+ down. Expanding the current SSA name in such further debug
+ uses by their RHS might lead to wrong debug info, as coalescing
+ might make the operands of such RHS be placed into the same
+ pseudo as something else. Like so:
+ a_1 = a_0 + 1; // Assume a_1 is TERed and a_0 is dead
+ use(a_1);
+ a_2 = ...
+ #DEBUG ... => a_1
+ As a_0 and a_2 don't overlap in lifetime, assume they are coalesced.
+ If we now would expand a_1 by it's RHS (a_0 + 1) in the debug use,
+ the write to a_2 would actually have clobbered the place which
+ formerly held a_0.
+
+ So, instead of that, we recognized the situation, and generate
+ debug temporaries at the last real use of TERed SSA names:
+ a_1 = a_0 + 1; // Assume a_1 is TERed and a_0 is dead
+ #DEBUG #D1 => a_1
+ use(a_1);
+ a_2 = ...
+ #DEBUG ... => #D1
+ */
+ if (MAY_HAVE_DEBUG_INSNS
+ && SA.values
+ && !is_gimple_debug (stmt))
+ {
+ ssa_op_iter iter;
+ tree op;
+ gimple def;
+
+ location_t sloc = get_curr_insn_source_location ();
+ tree sblock = get_curr_insn_block ();
+
+ /* Look for SSA names that have their last use here (TERed
+ names always have only one real use). */
+ FOR_EACH_SSA_TREE_OPERAND (op, stmt, iter, SSA_OP_USE)
+ if ((def = get_gimple_for_ssa_name (op)))
+ {
+ imm_use_iterator imm_iter;
+ use_operand_p use_p;
+ bool have_debug_uses = false;
+
+ FOR_EACH_IMM_USE_FAST (use_p, imm_iter, op)
+ {
+ if (gimple_debug_bind_p (USE_STMT (use_p)))
+ {
+ have_debug_uses = true;
+ break;
+ }
+ }
+
+ if (have_debug_uses)
+ {
+ /* OP is a TERed SSA name, with DEF it's defining
+ statement, and where OP is used in further debug
+ instructions. Generate a debug temporary, and
+ replace all uses of OP in debug insns with that
+ temporary. */
+ gimple debugstmt;
+ tree value = gimple_assign_rhs_to_tree (def);
+ tree vexpr = make_node (DEBUG_EXPR_DECL);
+ rtx val;
+ enum machine_mode mode;
+
+ set_curr_insn_source_location (gimple_location (def));
+ set_curr_insn_block (gimple_block (def));
+
+ DECL_ARTIFICIAL (vexpr) = 1;
+ TREE_TYPE (vexpr) = TREE_TYPE (value);
+ if (DECL_P (value))
+ mode = DECL_MODE (value);
+ else
+ mode = TYPE_MODE (TREE_TYPE (value));
+ DECL_MODE (vexpr) = mode;
+
+ val = gen_rtx_VAR_LOCATION
+ (mode, vexpr, (rtx)value, VAR_INIT_STATUS_INITIALIZED);
+
+ val = emit_debug_insn (val);
+
+ FOR_EACH_IMM_USE_STMT (debugstmt, imm_iter, op)
+ {
+ if (!gimple_debug_bind_p (debugstmt))
+ continue;
+
+ FOR_EACH_IMM_USE_ON_STMT (use_p, imm_iter)
+ SET_USE (use_p, vexpr);
+
+ update_stmt (debugstmt);
+ }
+ }
+ }
+ set_curr_insn_source_location (sloc);
+ set_curr_insn_block (sblock);
+ }
+
currently_expanding_gimple_stmt = stmt;
/* Expand this statement, then evaluate the resulting RTL and
*************** expand_gimple_basic_block (basic_block b
*** 3102,3107 ****
--- 3215,3227 ----
INSN_VAR_LOCATION_LOC (val) = (rtx)value;
}
+ /* In order not to generate too many debug temporaries,
+ we delink all uses of debug statements we already expanded.
+ Therefore debug statements between definition and real
+ use of TERed SSA names will continue to use the SSA name,
+ and not be replaced with debug temps. */
+ delink_stmt_imm_use (stmt);
+
gsi = nsi;
gsi_next (&nsi);
if (gsi_end_p (nsi))
Index: testsuite/gcc.dg/guality/pr43077-1.c
===================================================================
*** testsuite/gcc.dg/guality/pr43077-1.c (revision 0)
--- testsuite/gcc.dg/guality/pr43077-1.c (revision 0)
***************
*** 0 ****
--- 1,55 ----
+ /* PR debug/43077 */
+ /* { dg-do run } */
+ /* { dg-options "-g" } */
+
+ int varb;
+
+ int __attribute__((noinline))
+ fn1 (void)
+ {
+ int vara = (varb == 3); /* { dg-final { gdb-test 11 "vara" "0" } } */
+ asm volatile ("" : : "g" (vara)); /* { dg-final { gdb-test 11 "varb" "2" } } */
+ return 0;
+ }
+
+ int __attribute__((noinline))
+ fn2 (void)
+ {
+ int vara = (varb == 3); /* { dg-final { gdb-test 19 "vara" "1" } } */
+ asm volatile ("" : : "g" (vara)); /* { dg-final { gdb-test 19 "varb" "3" } } */
+ return 0;
+ }
+
+ int __attribute__((noinline))
+ foo (unsigned long *p, unsigned long *q)
+ {
+ int ret;
+ asm volatile ("" : "=r" (ret), "=r" (*p), "=r" (*q) : "0" (1), "1" (2), "2" (3));
+ return ret;
+ }
+
+ int __attribute__((noinline))
+ fn3 (void)
+ {
+ unsigned long a = 0, b = 0, c = 0;
+ a = foo (&b, &c);
+ /* { dg-final { gdb-test 42 "a" "1" } } */
+ /* { dg-final { gdb-test 42 "b" "2" } } */
+ /* { dg-final { gdb-test 42 "c" "3" } } */
+ unsigned long vara = a; /* { dg-final { gdb-test 42 "vara" "1" } } */
+ unsigned long varb = b; /* { dg-final { gdb-test 42 "varb" "2" } } */
+ unsigned long varc = c; /* { dg-final { gdb-test 42 "varc" "3" } } */
+ asm volatile ("" : : "g" (vara), "g" (varb), "g" (varc));
+ return a;
+ }
+
+ int
+ main (void)
+ {
+ asm volatile ("" : "=r" (varb) : "0" (2));
+ fn1 ();
+ asm volatile ("" : "=r" (varb) : "0" (3));
+ fn2 ();
+ fn3 ();
+ return 0;
+ }