Add support for nested blocks in Microsoft inline assembly

[• ehsan]

Summary

This fixes http://llvm.org/PR20204.

[majnemer]

Awesome!

Does this also glue together consecutive __asm statements that aren't in a
block?

Like:
asm mov 1, eax
asm add eax, ecx

That should turn into a single inline assembly IR instruction.

[majnemer]

LGTM with requested changes addressed.

lib/Parse/ParseStmtAsm.cpp

463

We should probably assert that LBraceLocs isn't empty at this point.

[• ehsan]

Landed in r212389 with the review comment addressed.

lib/Parse/ParseStmtAsm.cpp

 ///         ms-asm-line
 ///         ms-asm-line '\n' ms-asm-instruction-block
 ///
 StmtResult Parser::ParseMicrosoftAsmStatement(SourceLocation AsmLoc) {
   SourceManager &SrcMgr = PP.getSourceManager();
   SourceLocation EndLoc = AsmLoc;
   SmallVector<Token, 4> AsmToks;
 
-  bool InBraces = false;
+  unsigned BraceNesting = 0;
   unsigned short savedBraceCount = 0;
   bool InAsmComment = false;
   FileID FID;
   unsigned LineNo = 0;
   unsigned NumTokensRead = 0;
-  SourceLocation LBraceLoc;
+  SmallVector<SourceLocation, 4> LBraceLocs;
+  bool SkippedStartOfLine = false;
 
   if (Tok.is(tok::l_brace)) {
     // Braced inline asm: consume the opening brace.
-    InBraces = true;
+    BraceNesting = 1;
     savedBraceCount = BraceCount;
-    EndLoc = LBraceLoc = ConsumeBrace();
+    EndLoc = ConsumeBrace();
+    LBraceLocs.push_back(EndLoc);
     ++NumTokensRead;
   } else {
     // Single-line inline asm; compute which line it is on.
     std::pair<FileID, unsigned> ExpAsmLoc =
         SrcMgr.getDecomposedExpansionLoc(EndLoc);
     FID = ExpAsmLoc.first;
     LineNo = SrcMgr.getLineNumber(FID, ExpAsmLoc.second);
+    LBraceLocs.push_back(SourceLocation());
   }
 
   SourceLocation TokLoc = Tok.getLocation();
   do {
     // If we hit EOF, we're done, period.
     if (isEofOrEom())
       break;
 
-    if (!InAsmComment && Tok.is(tok::semi)) {
+    if (!InAsmComment && Tok.is(tok::l_brace)) {
+      // Consume the opening brace.
+      SkippedStartOfLine = Tok.isAtStartOfLine();
+      EndLoc = ConsumeBrace();
+      BraceNesting++;
+      LBraceLocs.push_back(EndLoc);
+      TokLoc = Tok.getLocation();
+      ++NumTokensRead;
+      continue;
+    } else if (!InAsmComment && Tok.is(tok::semi)) {
       // A semicolon in an asm is the start of a comment.
       InAsmComment = true;
-      if (InBraces) {
+      if (BraceNesting) {
         // Compute which line the comment is on.
         std::pair<FileID, unsigned> ExpSemiLoc =
             SrcMgr.getDecomposedExpansionLoc(TokLoc);
         FID = ExpSemiLoc.first;
         LineNo = SrcMgr.getLineNumber(FID, ExpSemiLoc.second);
       }
-    } else if (!InBraces || InAsmComment) {
+    } else if (!BraceNesting || InAsmComment) {
       // If end-of-line is significant, check whether this token is on a
       // new line.
       std::pair<FileID, unsigned> ExpLoc =
           SrcMgr.getDecomposedExpansionLoc(TokLoc);
       if (ExpLoc.first != FID ||
           SrcMgr.getLineNumber(ExpLoc.first, ExpLoc.second) != LineNo) {
         // If this is a single-line __asm, we're done.
-        if (!InBraces)
+        if (!BraceNesting)
           break;
         // We're no longer in a comment.
         InAsmComment = false;
       } else if (!InAsmComment && Tok.is(tok::r_brace)) {
         // Single-line asm always ends when a closing brace is seen.
         // FIXME: This is compatible with Apple gcc's -fasm-blocks; what
         // does MSVC do here?
         break;
       }
     }
-    if (!InAsmComment && InBraces && Tok.is(tok::r_brace) &&
-        BraceCount == (savedBraceCount + 1)) {
-      // Consume the closing brace, and finish
+    if (!InAsmComment && BraceNesting && Tok.is(tok::r_brace) &&
+        BraceCount == (savedBraceCount + BraceNesting)) {
+      // Consume the closing brace.
+      SkippedStartOfLine = Tok.isAtStartOfLine();
       EndLoc = ConsumeBrace();
+      BraceNesting--;
+      // Finish if all of the opened braces in the inline asm section were consumed.
+      if (BraceNesting == 0)
         break;
+      else {
+        LBraceLocs.pop_back();
+        TokLoc = Tok.getLocation();
+        ++NumTokensRead;
+        continue;
+      }
     }
 
     // Consume the next token; make sure we don't modify the brace count etc.
     // if we are in a comment.
     EndLoc = TokLoc;
     if (InAsmComment)
       PP.Lex(Tok);
     else {
+      // Set the token as the start of line if we skipped the original start
+      // of line token in case it was a nested brace.
+      if (SkippedStartOfLine)
+        Tok.setFlag(Token::StartOfLine);
       AsmToks.push_back(Tok);
       ConsumeAnyToken();
     }
     TokLoc = Tok.getLocation();
     ++NumTokensRead;
+    SkippedStartOfLine = false;
   } while (1);
 
-  if (InBraces && BraceCount != savedBraceCount) {
+  if (BraceNesting && BraceCount != savedBraceCount) {
     // __asm without closing brace (this can happen at EOF).
+    for (unsigned i = 0; i < BraceNesting; ++i) {
       Diag(Tok, diag::err_expected) << tok::r_brace;
-    Diag(LBraceLoc, diag::note_matching) << tok::l_brace;
+      Diag(LBraceLocs.back(), diag::note_matching) << tok::l_brace;
+      LBraceLocs.pop_back();
+    }
     return StmtError();
   } else if (NumTokensRead == 0) {
     // Empty __asm.
     Diag(Tok, diag::err_expected) << tok::l_brace;
     return StmtError();
   }
 
   // Okay, prepare to use MC to parse the assembly.
StmtResult Parser::ParseMicrosoftAsmStatement(SourceLocation AsmLoc) {
   if (UnsupportedArch) {
     Diag(AsmLoc, diag::err_msasm_unsupported_arch) << TheTriple.getArchName();
   } else {
     std::string Error;
     TheTarget = llvm::TargetRegistry::lookupTarget(TT, Error);
     if (!TheTarget)
       Diag(AsmLoc, diag::err_msasm_unable_to_create_target) << Error;
   }
 
   // If we don't support assembly, or the assembly is empty, we don't
   // need to instantiate the AsmParser, etc.
   if (!TheTarget || AsmToks.empty()) {
-    return Actions.ActOnMSAsmStmt(AsmLoc, LBraceLoc, AsmToks, StringRef(),
+    return Actions.ActOnMSAsmStmt(AsmLoc, LBraceLocs[0], AsmToks, StringRef(),
                                   /*NumOutputs*/ 0, /*NumInputs*/ 0,
                                   ConstraintRefs, ClobberRefs, Exprs, EndLoc);
   }
 
   // Expand the tokens into a string buffer.
   SmallString<512> AsmString;
   SmallVector<unsigned, 8> TokOffsets;
   if (buildMSAsmString(PP, AsmLoc, AsmToks, TokOffsets, AsmString))
if (OpExprs[i].second)
       OpExpr =
           Actions.BuildUnaryOp(getCurScope(), AsmLoc, UO_AddrOf, OpExpr).get();
 
     ConstraintRefs[i] = StringRef(Constraints[i]);
     Exprs[i] = OpExpr;
   }
 
   // FIXME: We should be passing source locations for better diagnostics.
-  return Actions.ActOnMSAsmStmt(AsmLoc, LBraceLoc, AsmToks, AsmStringIR,
+  return Actions.ActOnMSAsmStmt(AsmLoc, LBraceLocs[0], AsmToks, AsmStringIR,
                                 NumOutputs, NumInputs, ConstraintRefs,
                                 ClobberRefs, Exprs, EndLoc);
 }
 
 /// ParseAsmStatement - Parse a GNU extended asm statement.
 ///       asm-statement:
 ///         gnu-asm-statement
 ///         ms-asm-statement

test/CodeGen/ms-inline-asm-64.c

 }
 
 int t4() {
   struct t3_type foo;
   foo.a = 1;
   foo.b = 2;
   __asm {
      lea ebx, foo
+     {
        mov eax, [ebx].foo.a
+     }
      mov [ebx].foo.b, ecx
   }
   return foo.b;
 // CHECK: t4
 // CHECK: call void asm sideeffect inteldialect "lea ebx, qword ptr $0\0A\09mov eax, [ebx].0\0A\09mov [ebx].4, ecx", "*m,~{eax},~{ebx},~{dirflag},~{fpsr},~{flags}"(%struct.t3_type* %{{.*}})
 }

test/CodeGen/ms-inline-asm.c

 // CHECK: t6
 // CHECK: call void asm sideeffect inteldialect "int $$0x2c", "~{dirflag},~{fpsr},~{flags}"()
 }
 
 void t7() {
   __asm {
     int 0x2c ; } asm comments are fun! }{
   }
+  __asm {
+    {
+      int 0x2c ; } asm comments are fun! }{
+    }
+  }
   __asm {}
 // CHECK: t7
 // CHECK: call void asm sideeffect inteldialect "int $$0x2c", "~{dirflag},~{fpsr},~{flags}"()
 // CHECK: call void asm sideeffect inteldialect "", "~{dirflag},~{fpsr},~{flags}"()
 }
 
 int t8() {
   __asm int 4 ; } comments for single-line asm
   __asm {}
   __asm int 4
   return 10;
 // CHECK: t8
 // CHECK: call void asm sideeffect inteldialect "int $$4", "~{dirflag},~{fpsr},~{flags}"()
 // CHECK: call void asm sideeffect inteldialect "", "~{dirflag},~{fpsr},~{flags}"()
 // CHECK: call void asm sideeffect inteldialect "int $$4", "~{dirflag},~{fpsr},~{flags}"()
 // CHECK: ret i32 10
 }
 
 void t9() {
   __asm {
     push ebx
-    mov ebx, 0x07
-    pop ebx
+    { mov ebx, 0x07 }
+    __asm { pop ebx }
   }
 // CHECK: t9
 // CHECK: call void asm sideeffect inteldialect "push ebx\0A\09mov ebx, $$0x07\0A\09pop ebx", "~{ebx},~{esp},~{dirflag},~{fpsr},~{flags}"()
 }
 
 unsigned t10(void) {
   unsigned i = 1, j;
   __asm {

 // CHECK: call void asm sideeffect inteldialect "movzx eax, byte ptr $0", "*m,~{eax},~{dirflag},~{fpsr},~{flags}"(i8* %{{.*}})
 // CHECK: call void asm sideeffect inteldialect "movzx eax, word ptr $0", "*m,~{eax},~{dirflag},~{fpsr},~{flags}"(i16* %{{.*}})
 }
 
 void t14() {
   unsigned i = 1, j = 2;
   __asm {
     .if 1
-    mov eax, i
+    { mov eax, i }
     .else
     mov ebx, j
     .endif
   }
 // CHECK: t14
 // CHECK: call void asm sideeffect inteldialect ".if 1\0A\09mov eax, dword ptr $0\0A\09.else\0A\09mov ebx, j\0A\09.endif", "*m,~{eax},~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}})
 }
 

test/Parser/ms-inline-asm-nested-braces.c

+// REQUIRES: x86-registered-target
+// RUN: %clang_cc1 %s -triple i386-apple-darwin10 -verify -fasm-blocks
+
+int t_fail() { // expected-note {{to match this}}
+  __asm
+  { // expected-note {{to match this}}
+    { // expected-note {{to match this}}
+      {
+      } // expected-error 3 {{expected}}

test/Parser/ms-inline-asm.c

void t7() {
   }
 }
 void t8() {
   __asm nop __asm nop __asm nop
 }
 void t9() {
   __asm nop __asm nop ; __asm nop
 }
+void t10() {
+  __asm {
+    mov eax, 0
+    __asm {
+      mov eax, 1
+      {
+        mov eax, 2
+      }
+    }
+  }
+}
 int t_fail() { // expected-note {{to match this}}
   __asm 
   __asm { // expected-error 3 {{expected}} expected-note {{to match this}}