On 2020-03-06 09:55, Richard Biener wrote:
On Thu, Mar 5, 2020 at 5:49 PM J.W. Jagersma <jwjager...@gmail.com> wrote:diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c index 2a409dcaffe..8314db00922 100644 --- a/gcc/tree-eh.c +++ b/gcc/tree-eh.c @@ -2077,6 +2077,9 @@ lower_eh_constructs_2 (struct leh_state *state, gimple_stmt_iterator *gsi) DECL_GIMPLE_REG_P (tmp) = 1; gsi_insert_after (gsi, s, GSI_SAME_STMT); } + /* FALLTHRU */ + + case GIMPLE_ASM: /* Look for things that can throw exceptions, and record them. */ if (state->cur_region && stmt_could_throw_p (cfun, stmt)) {The part you skip here for ASMs means that for ASM outputs we do not represent their values correctly on EH edges (and we eventually never will be able to since we don't know exactly at which point the actual assembly throws). So I think this "feature" needs to be documented appropriately and eventually only ASMs without outputs made allowed to throw?
So the code for assign stmts above would have to be duplicated and modified so that each output gets assigned to a temporary. I'll see if I can do that. Personally I think it's reasonable to assume that if an asm throws, its operands won't be modified (ie. it contains only a single instruction). But there is no way to enforce this from the compiler, of course.
diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c index 6528acac31a..03bc1d52cfa 100644 --- a/gcc/tree-into-ssa.c +++ b/gcc/tree-into-ssa.c @@ -1415,7 +1415,43 @@ rewrite_stmt (gimple_stmt_iterator *si) if (tracked_var) { gimple *note = gimple_build_debug_bind (tracked_var, name, stmt); - gsi_insert_after (si, note, GSI_SAME_STMT); + /* If stmt ends the bb, insert the debug stmt on the single + non-EH edge from the stmt. */ + if (gsi_one_before_end_p (*si) && stmt_ends_bb_p (stmt)) + { + basic_block bb = gsi_bb (*si); + edge_iterator ei; + edge e, ef = NULL; + FOR_EACH_EDGE (e, ei, bb->succs) + if (!(e->flags & EDGE_EH))I think this needs to check for abnormal edges as well to cover the case of i = setjmp (); which means doing !(e->flags & EDGE_COMPLEX) and adjusting the comment.+ { + gcc_checking_assert (!ef); + 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. + + 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); + }it would be nice to elide building of the debug bind if we don't end up inserting it.
Then this and the above would have to be changed in maybe_register_def too? That is where I copied this code from.
Otherwise the patch looks reasonable. It lacks a testcase though, I think a target specific testcase can be constructed easily.
Non-call-exceptions need to be supported by the OS and/or runtime library though. I rolled my own but I don't know on which platforms (if any) this is available by default.
