On Thu, 15 Dec 2022, Jakub Jelinek wrote:
> Hi!
>
> The following testcase ICEs, because ccp1 replaced
> s.0_1 = &s;
> __asm__ goto("" : "=r" MEM[(T *)s.0_1] : : : "lab" lab);
> with
> __asm__ goto("" : "=r" s : : : "lab" lab);
> and because s is no longer addressable, we are rewriting it into
> ssa and want
> __asm__ goto("" : "=r" s_7 : : : "lab" lab);
> plus debug stmt
> # DEBUG s => s_7
> The code assumes that there is at most one non-EH edge in that
> case, but with the addition of outputs to asm goto that is no longer the
> case, we can have many outgoing edges.
>
> The patch keeps the checking assertion that there is at most one such
> edge for everything but asm goto, but moves the addition of the debug
> stmt into the loop, so that it can be added on all edges where it is
> possible, not just one of them.
>
> Furthermore, looking at gsi_insert_on_edge_immediate
> -> gimple_find_edge_insert_loc, the conditions to insert stmt there
> to the destination block are
> if (single_pred_p (dest)
> && gimple_seq_empty_p (phi_nodes (dest))
> && dest != EXIT_BLOCK_PTR_FOR_FN (cfun))
> (plus there is code to insert it in the previous block but that is
> never true when the pred is known to be stmt_ends_bb_p), while
> mayube_register_def was just checking
> if (ef && single_pred_p (ef->dest)
> && ef->dest != EXIT_BLOCK_PTR_FOR_FN (cfun))
> so if for whatever reason ef->dest had any PHIs, we'd split the
> edge for -g and not for -g0, something we must avoid for -fcompare-debug
> stability. So, I've added the no phi_nodes check too.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Thanks,
Richard.
> 2022-12-15 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/108095
> * tree-into-ssa.cc (maybe_register_def): Insert debug stmt
> on all non-EH edges from asm goto if they have a single
> predecessor rather than asserting there is at most one such edge.
> Test whether there are no PHI nodes next to the single predecessor
> test.
>
> * gcc.dg/pr108095.c: New test.
>
> --- gcc/tree-into-ssa.cc.jj 2022-12-05 23:20:24.000000000 +0100
> +++ gcc/tree-into-ssa.cc 2022-12-14 14:20:50.301283097 +0100
> @@ -1934,9 +1934,8 @@ maybe_register_def (def_operand_p def_p,
> tree tracked_var = target_for_debug_bind (sym);
> if (tracked_var)
> {
> - gimple *note = gimple_build_debug_bind (tracked_var, def, stmt);
> - /* If stmt ends the bb, insert the debug stmt on the single
> - non-EH edge from the stmt. */
> + /* If stmt ends the bb, insert the debug stmt on the non-EH
> + edge(s) from the stmt. */
> if (gsi_one_before_end_p (gsi) && stmt_ends_bb_p (stmt))
> {
> basic_block bb = gsi_bb (gsi);
> @@ -1945,33 +1944,46 @@ maybe_register_def (def_operand_p def_p,
> FOR_EACH_EDGE (e, ei, bb->succs)
> if (!(e->flags & EDGE_EH))
> {
> - gcc_checking_assert (!ef);
> + /* asm goto can have multiple non-EH edges from the
> + stmt. Insert on all of them where it is
> + possible. */
> + gcc_checking_assert (!ef || (gimple_code (stmt)
> + == GIMPLE_ASM));
> ef = e;
> - }
> - /* If there are other predecessors to ef->dest, then
> - there must be PHI nodes for the modified
> - variable, and therefore there will be debug bind
> - stmts after the PHI nodes. The debug bind notes
> - we'd insert would force the creation of a new
> - block (diverging codegen) and be redundant with
> - the post-PHI bind stmts, so don't add them.
> + /* If there are other predecessors to ef->dest, then
> + there must be PHI nodes for the modified
> + variable, and therefore there will be debug bind
> + stmts after the PHI nodes. The debug bind notes
> + we'd insert would force the creation of a new
> + block (diverging codegen) and be redundant with
> + the post-PHI bind stmts, so don't add them.
>
> - As for the exit edge, there wouldn't be redundant
> - bind stmts, but there wouldn't be a PC to bind
> - them to either, so avoid diverging the CFG. */
> - if (ef && single_pred_p (ef->dest)
> - && ef->dest != EXIT_BLOCK_PTR_FOR_FN (cfun))
> - {
> - /* If there were PHI nodes in the node, we'd
> - have to make sure the value we're binding
> - doesn't need rewriting. But there shouldn't
> - be PHI nodes in a single-predecessor block,
> - so we just add the note. */
> - gsi_insert_on_edge_immediate (ef, note);
> - }
> + As for the exit edge, there wouldn't be redundant
> + bind stmts, but there wouldn't be a PC to bind
> + them to either, so avoid diverging the CFG. */
> + if (e
> + && single_pred_p (e->dest)
> + && gimple_seq_empty_p (phi_nodes (e->dest))
> + && e->dest != EXIT_BLOCK_PTR_FOR_FN (cfun))
> + {
> + /* If there were PHI nodes in the node, we'd
> + have to make sure the value we're binding
> + doesn't need rewriting. But there shouldn't
> + be PHI nodes in a single-predecessor block,
> + so we just add the note. */
> + gimple *note
> + = gimple_build_debug_bind (tracked_var, def,
> + stmt);
> + gsi_insert_on_edge_immediate (ef, note);
> + }
> + }
> }
> else
> - gsi_insert_after (&gsi, note, GSI_SAME_STMT);
> + {
> + gimple *note
> + = gimple_build_debug_bind (tracked_var, def, stmt);
> + gsi_insert_after (&gsi, note, GSI_SAME_STMT);
> + }
> }
> }
>
> --- gcc/testsuite/gcc.dg/pr108095.c.jj 2022-12-14 14:31:57.206580191
> +0100
> +++ gcc/testsuite/gcc.dg/pr108095.c 2022-12-14 14:31:25.234045359 +0100
> @@ -0,0 +1,15 @@
> +/* PR tree-optimization/108095 */
> +/* { dg-do compile } */
> +/* { dg-options "-Os -g" } */
> +
> +int v;
> +typedef unsigned T;
> +
> +void
> +foo (void)
> +{
> + unsigned s;
> + asm goto ("" : "=r" (*(T *) &s) : : : lab);
> + v = 0;
> +lab:
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
