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