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

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 marked 7 inline comments as done.
jyu2 added inline comments.


================
Comment at: lib/CodeGen/CGStmt.cpp:2182
+    }
+  }
+
----------------
nickdesaulniers wrote:
> If this new block was moved closer to the new one on L2227, I assume they could be combined and possibly `IsGCCAsmGoto` be removed?  The code currently in between doesn't appear at first site to depend on info from this block, though maybe I may be missing it.
The labels need be processed before Clobbers


================
Comment at: lib/Parse/ParseStmtAsm.cpp:830-858
+  if (AteExtraColon || Tok.is(tok::colon)) {
+    if (AteExtraColon)
+      AteExtraColon = false;
+    else
+      ConsumeToken();
+
+    if (!AteExtraColon && Tok.isNot(tok::identifier)) {
----------------
nickdesaulniers wrote:
> ```
> if (x || y) {
>   if (x) foo();
>   else bar();
>   if (!x && ...) baz();
>   if (!x && ...) quux();
> ```
> is maybe more readable as:
> ```
> if (x) foo();
> else if (y)
>   bar();
>   baz();
>   quux();
> ```
This is better?


================
Comment at: lib/Sema/SemaStmtAsm.cpp:470
+    if (NS->isGCCAsmGoto() &&
+        Exprs[ConstraintIdx]->getStmtClass() == Stmt::AddrLabelExprClass)
+      break;
----------------
efriedma wrote:
> This looks suspicious; an AddrLabelExpr could be an input or output, e.g. `"r"(&&foo)`.
Syntax for asm goto:
 Syntax:
   asm [volatile] goto ( AssemblerTemplate
                       :
                       : InputOperands
                       : Clobbers
                       : GotoLabels)

 Only input is allowed.  Output is not allowed



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

https://reviews.llvm.org/D56571