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

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Thanks for all the work that went into this patch, both initially and rebasing!



================
Comment at: lib/AST/Stmt.cpp:465
+  const LabelDecl *LD = getLabelExpr(i)->getLabel();
+  return LD->getName();
+}
----------------
These 2 statements can probably be condensed into one.
```
return getLabelExpr(i)->getLabel()->getName();
```


================
Comment at: lib/CodeGen/CGStmt.cpp:2182
+    }
+  }
+
----------------
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.


================
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)) {
----------------
```
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();
```


================
Comment at: test/CodeGen/asm.c:271
+  // CHECK: callbr void asm sideeffect "testl $0, $0; jne ${1:l};", "r,X,X,~{dirflag},~{fpsr},~{flags}"(i32 %0, i8* blockaddress(@t32, %label_true), i8* blockaddress(@t32, %loop)) #1
+    return 0;
+loop:
----------------
extra indentation?


================
Comment at: test/Parser/asm.c:22-37
+    // expected-error at +1 {{expected ')'}}
+    asm ("mov %[e], %[e]" : : [e] "rm" (*e)::a)
+    // expected-error at +1 {{expected ':'}}
+    asm goto ("decl %0; jnz %l[a]" :"=r"(x): "m"(x) : "memory" : a);
+    // expected-error at +1 {{expected identifie}}
+    asm goto ("decl %0;" :: "m"(x) : "memory" : );
+    // expected-error at +1 {{expected ':'}} 
----------------
Extra indentation intentional?


================
Comment at: test/Parser/asm.cpp:14-29
+    // expected-error at +1 {{expected ')'}}
+    asm ("mov %[e], %[e]" : : [e] "rm" (*e)::a)
+    // expected-error at +1  {{expected ':'}}
+    asm goto ("decl %0; jnz %l[a]" :"=r"(x): "m"(x) : "memory" : a);
+    // expected-error at +1 {{expected identifie}}
+    asm goto ("decl %0;" :: "m"(x) : "memory" : );
+    // expected-error at +1  {{expected ':'}}
----------------
ditto


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

https://reviews.llvm.org/D56571