[PATCH] Implementation of asm goto outputs

[Vladimir Makarov]

On 2020-11-13 4:00 a.m., Richard Biener wrote:
> 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".
>
Yes, it has sense.  A regular user hardly knows our internal kitchen.
> 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;

Richard, thank you for your review.  I am not familiar well with the 
middle-end.  So your comments are really useful.

> I wonder why the code never ran into issues for calls that throw
> internal ...


I have no idea.  But I really ran into this problem when I tested asm 
goto with outputs.

> You have plenty compile testcases but not a single execute one.
> So - does it actually work? ;)
Yes, it works.  Two tests actually produces output reloads at least on 
x86-64.  As for execution tests, it is difficult to write for me 
something meaningful, especially with generated output reloads. I'll try 
to add execution tests too.
> Otherwise OK.


Richard, thank you again for your quick review. I'll update the patch 
according to your proposals, test it again and commit it.


>> 2020-11-12  Vladimir Makarov <vmakarov@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


OK thanks.


> 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  <vmakarov@redhat.com>
>>
>>           * c-c++-common/asmgoto-2.c: Permit output in asm goto.
>>           * gcc.c-torture/compile/asmgoto-[2345].c: New tests.
>>