<div dir="auto">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.<div dir="auto"><br></div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Feb 5, 2019, 3:32 AM Richard Smith - zygoloid via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">rsmith added inline comments.<br>
<br>
<br>
================<br>
Comment at: lib/AST/Stmt.cpp:457-460<br>
   this->NumOutputs = NumOutputs;<br>
   this->NumInputs = NumInputs;<br>
   this->NumClobbers = NumClobbers;<br>
+  this->NumLabels = NumLabels;<br>
----------------<br>
jyu2 wrote:<br>
> rsmith wrote:<br>
> > Please assert that you don't have both outputs and labels here. (If you did, you would assign them the same slots within `Exprs`.)<br>
> > <br>
> > 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.)<br>
> This is enforcement during the parer.  <br>
> <br>
> for example:<br>
> a.c:12:23: error: expected ':'<br>
> asm goto ("decl %0;" :"a": "m"(cond) : "memory" );<br>
> <br>
> Do you think this is enough for now?<br>
> Thanks.<br>
> Jennifer<br>
Thanks, I see it now. Please still add the assert here.<br>
<br>
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.<br>
<br>
<br>
CHANGES SINCE LAST ACTION<br>
  <a href="https://reviews.llvm.org/D56571/new/" rel="noreferrer noreferrer" target="_blank">https://reviews.llvm.org/D56571/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D56571" rel="noreferrer noreferrer" target="_blank">https://reviews.llvm.org/D56571</a><br>
<br>
<br>
<br>
</blockquote></div>