On Thu, Nov 12, 2020 at 8:55 PM Vladimir Makarov via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > The following patch implements asm goto with outputs. Kernel > developers several times expressed wish to have this feature. Asm > goto with outputs was implemented in LLVM recently. This new feature > was presented on 2020 linux plumbers conference > (https://linuxplumbersconf.org/event/7/contributions/801/attachments/659/1212/asm_goto_w__Outputs.pdf) > and 2020 LLVM conference > (https://www.youtube.com/watch?v=vcPD490s-hE). > > The patch permits to use outputs in asm gotos only when LRA is used. > It is problematic to implement it in the old reload pass. To be > honest it was hard to implement it in LRA too until global live info > update was added to LRA few years ago. > > Different from LLVM asm goto output implementation, you can use > outputs on any path from the asm goto (not only on fallthrough path as > in LLVM). > > The patch removes critical edges on which potentially asm output > reloads could occur (it means you can have several asm gotos using the > same labels and the same outputs). It is done in IRA as it is > difficult to create new BBs in LRA. The most of the work (placement > of output reloads in BB destinations of asm goto basic block) is done in > LRA. When it happens, LRA updates global live info to reflect that > new pseudos live on the BB borders and the old ones do not live there > anymore. > > I tried also approach to split live ranges of pseudos involved in > asm goto outputs to guarantee they get hard registers in IRA. But > this approach did not work as it is difficult to keep this assignment > through all LRA. Also probably it would result in worse code as move > insn coalescing is not guaranteed. > > Asm goto with outputs will not work for targets which were not > converted to LRA (probably some outdated targets as the old reload > pass is not supported anymore). An error will be generated when the > old reload pass meets asm goto with an output. A precaution is taken > not to crash compiler after this error. > > The patch is pretty small as all necessary infrastructure was > already implemented, practically in all compiler pipeline. It did not > required adding new RTL insns opposite to what Google engineers did to > LLVM MIR. > > The patch could be also useful for implementing jump insns with > output reloads in the future (e.g. branch and count insns). > > I think asm gotos with outputs should be considered as an experimental > feature as there are no real usage of this yet. Earlier adoption of > this feature could help with debugging and hardening the > implementation. > > The patch was successfully bootstrapped and tested on x86-64, ppc64, > and aarch64. > > Are non-RA changes ok in the patch?
Minor nit for the RA parts: + if (i < recog_data.n_operands) + { + error_for_asm (insn, + "old reload pass does not support asm goto " + "with outputs in %<asm%>"); + ira_nullify_asm_goto (insn); I'd say "the target does not support ...", the user shouldn't be concerned about a thing called "reload". diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c index 1493b323956..9be8e295627 100644 --- a/gcc/tree-into-ssa.c +++ b/gcc/tree-into-ssa.c @@ -1412,6 +1412,11 @@ rewrite_stmt (gimple_stmt_iterator *si) SET_DEF (def_p, name); register_new_def (DEF_FROM_PTR (def_p), var); + /* Do not insert debug stmt after asm goto: */ + if (gimple_code (stmt) == GIMPLE_ASM + && gimple_asm_nlabels (as_a <gasm *> (stmt)) > 0) + continue; + why? Ah, the next line explains. I guess it's better done as /* Do not insert debug stmts if the stmt ends the BB. */ if (stmt_ends_bb_p (stmt)) continue; I wonder why the code never ran into issues for calls that throw internal ... You have plenty compile testcases but not a single execute one. So - does it actually work? ;) Otherwise OK. > 2020-11-12 Vladimir Makarov <vmaka...@redhat.com> > > * c/c-parser.c (c_parser_asm_statement): Parse outputs for asm > goto too. > * c/c-typeck.c (build_asm_expr): Remove an assert checking output > absence for asm goto. I'm sure this will be rejected by the commit hook. You need sth like gcc/c/ * c-parser.c (... gcc/ > * cfgexpand.c (expand_asm_stmt): Output asm goto with outputs too. > Place insns after asm goto on edges. > * cp/parser.c (cp_parser_asm_definition): Parse outputs for asm > goto too. > * doc/extend.texi: Reflect the changes in asm goto documentation. > * gcc/gimple.c (gimple_build_asm_1): Remove an assert checking > output > absence for asm goto. > * gimple.h (gimple_asm_label_op, gimple_asm_set_label_op): Take > possible asm goto outputs into account. > * ira.c (ira): Remove critical edges for potential asm goto output > reloads. > (ira_nullify_asm_goto): New function. > * ira.h (ira_nullify_asm_goto): New prototype. > * lra-assigns.c (lra_split_hard_reg_for): Use ira_nullify_asm_goto. > Check that splitting is done inside a basic block. > * lra-constraints.c (curr_insn_transform): Permit output reloads > for any jump insn. > * lra-spills.c (lra_final_code_change): Remove USEs added in > ira for asm gotos. > * lra.c (lra_process_new_insns): Place output reload insns after > jumps in the beginning of destination BBs. > * reload.c (find_reloads): Report error for asm gotos with > outputs. Modify them to keep CFG consistency to avoid crashes. > * tree-into-ssa.c (rewrite_stmt): Don't put debug stmt after asm > goto. > > > 2020-11-12 Vladimir Makarov <vmaka...@redhat.com> > > * c-c++-common/asmgoto-2.c: Permit output in asm goto. > * gcc.c-torture/compile/asmgoto-[2345].c: New tests. >