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.
>

Reply via email to