[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Eli Friedman via cfe-commits]

What sort of outputs, specifically?  Memory outputs are easy; I think the LLVM IR patch actually supports them already.  Register outputs are complicated... I guess they're also possible, but we would need to come up with some frontend rule that allows sane code generation.  Consider the case where two or more asm gotos jump to the same destination, and output a value to the same register, but a different variable.  In general, it's possible to lower by rewriting the PHI nodes to match, then inserting a switch at the beginning of the destination to fix the rewritten PHIs.  But that's both expensive at runtime and difficult to implement, so we probably want a rule that prohibits constructs which would require that sort of lowering.


If we do intend to implement register outputs at some point, we might want to draft a design at the LLVM IR level now, so we have some confidence the current IR is forward-compatible.


Obviously we should have diagnostics in the parser if the user tries to write an asm goto with an output.  I'm not worried about the difficulty of fixing the frontend otherwise; this patch isn't that big in the first place.


-Eli


________________________________
From: Nick Desaulniers <ndesaulniers at google.com>
Sent: Tuesday, February 5, 2019 1:14 AM
To: reviews+D56571+public+1482c3abd76a8c19 at reviews.llvm.org
Cc: Yu, Jennifer; Keane, Erich; chandlerc at gmail.com; syaghmour at apple.com; craig.topper at gmail.com; hans at chromium.org; e5ten.arch at gmail.com; dima at golovin.in; natechancellor at gmail.com; tstellar at redhat.com; Eli Friedman; srhines at google.com; eraman at google.com; cfe-commits at lists.llvm.org
Subject: [EXT] Re: [PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

Note that kernel developers are requesting output support with ASM goto. Doesn't need to be in V1 of ASM go-to support, but we should expect it to be implemented in the future. We should help users with diagnostics that this is not supported, and add asserts that will help us implement such a feature in the future.


On Tue, Feb 5, 2019, 3:32 AM Richard Smith - zygoloid via Phabricator <reviews at reviews.llvm.org<mailto:reviews at reviews.llvm.org> wrote:
rsmith added inline comments.


================
Comment at: lib/AST/Stmt.cpp:457-460
   this->NumOutputs = NumOutputs;
   this->NumInputs = NumInputs;
   this->NumClobbers = NumClobbers;
+  this->NumLabels = NumLabels;
----------------
jyu2 wrote:
> rsmith wrote:
> > Please assert that you don't have both outputs and labels here. (If you did, you would assign them the same slots within `Exprs`.)
> >
> > You also seem to be missing `Sema` enforcement of the rule that you cannot have both outputs and labels. (If you want to actually support that as an intentional extension to the GCC functionality, you should change the implementation of `GCCAsmStmt` to use different slots for outputs and labels, add some tests, and add a `-Wgcc-compat` warning for that case. Seems better to just add an error for it for now.)
> This is enforcement during the parer.
>
> for example:
> a.c:12:23: error: expected ':'
> asm goto ("decl %0;" :"a": "m"(cond) : "memory" );
>
> Do you think this is enough for now?
> Thanks.
> Jennifer
Thanks, I see it now. Please still add the assert here.

I'd also like a custom diagnostic for that parse error; it'd seem easy and useful to add an "asm goto cannot have output constraints" error.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56571/new/

https://reviews.llvm.org/D56571



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190205/1961c339/attachment.html>