On Thu, Nov 12, 2020 at 8:55 PM Vladimir Makarov via Gcc-patches
<[email protected]> 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 <[email protected]>
>
> * 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 <[email protected]>
>
> * c-c++-common/asmgoto-2.c: Permit output in asm goto.
> * gcc.c-torture/compile/asmgoto-[2345].c: New tests.
>
