[cfe-commits] r161890 - in /cfe/trunk: lib/Parse/ParseStmt.cpp test/CodeGen/ms-inline-asm.c test/Parser/ms-inline-asm.c

Chad Rosier mcrosier at apple.com
Tue Aug 14 12:25:35 PDT 2012


For the record the commit message should have been:

[ms-inline asm] Fix the parser so that it does _not_ merge adjacent ms-style
inline assembly statements.  Richard pointed out that we were misparsing
things such as:

  if (foo())
    __asm nop
  __asm nop

Basically, the second nop was being merged with the first and was therefore
being predicated on the result of foo().  This is not consistent with how
MSVC behaves.

 Chad

On Aug 14, 2012, at 12:22 PM, Chad Rosier wrote:

> Author: mcrosier
> Date: Tue Aug 14 14:22:06 2012
> New Revision: 161890
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=161890&view=rev
> Log:
> [ms-inline asm] Add a helpful assert.
> 
> Modified:
>    cfe/trunk/lib/Parse/ParseStmt.cpp
>    cfe/trunk/test/CodeGen/ms-inline-asm.c
>    cfe/trunk/test/Parser/ms-inline-asm.c
> 
> Modified: cfe/trunk/lib/Parse/ParseStmt.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=161890&r1=161889&r2=161890&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParseStmt.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseStmt.cpp Tue Aug 14 14:22:06 2012
> @@ -1658,108 +1658,102 @@
>   SourceLocation EndLoc = AsmLoc;
>   SmallVector<Token, 4> AsmToks;
>   SmallVector<unsigned, 4> LineEnds;
> +
> +  bool InBraces = false;
> +  unsigned short savedBraceCount = 0;
> +  bool InAsmComment = false;
> +  FileID FID;
> +  unsigned LineNo = 0;
> +  unsigned NumTokensRead = 0;
> +  SourceLocation LBraceLoc;
> +
> +  if (Tok.is(tok::l_brace)) {
> +    // Braced inline asm: consume the opening brace.
> +    InBraces = true;
> +    savedBraceCount = BraceCount;
> +    EndLoc = LBraceLoc = ConsumeBrace();
> +    ++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);
> +  }
> +
> +  SourceLocation TokLoc = Tok.getLocation();
>   do {
> -    bool InBraces = false;
> -    unsigned short savedBraceCount = 0;
> -    bool InAsmComment = false;
> -    FileID FID;
> -    unsigned LineNo = 0;
> -    unsigned NumTokensRead = 0;
> -    SourceLocation LBraceLoc;
> -
> -    if (Tok.is(tok::l_brace)) {
> -      // Braced inline asm: consume the opening brace.
> -      InBraces = true;
> -      savedBraceCount = BraceCount;
> -      EndLoc = LBraceLoc = ConsumeBrace();
> -      ++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);
> -    }
> -
> -    SourceLocation TokLoc = Tok.getLocation();
> -    do {
> -      // If we hit EOF, we're done, period.
> -      if (Tok.is(tok::eof))
> -        break;
> +    // If we hit EOF, we're done, period.
> +    if (Tok.is(tok::eof))
> +      break;
> 
> -      // The asm keyword is a statement separator, so multiple asm statements
> -      // are allowed.
> -      if (!InAsmComment && Tok.is(tok::kw_asm))
> -        break;
> +    // The asm keyword is a statement separator, so multiple asm statements
> +    // are allowed on a single line.
> +    if (!InAsmComment && Tok.is(tok::kw_asm))
> +      break;
> 
> -      if (!InAsmComment && Tok.is(tok::semi)) {
> -        // A semicolon in an asm is the start of a comment.
> -        InAsmComment = true;
> -        if (InBraces) {
> -          // 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) {
> -        // 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)
> -            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 && Tok.is(tok::semi)) {
> +      // A semicolon in an asm is the start of a comment.
> +      InAsmComment = true;
> +      if (InBraces) {
> +        // Compute which line the comment is on.
> +        std::pair<FileID, unsigned> ExpSemiLoc =
> +          SrcMgr.getDecomposedExpansionLoc(TokLoc);
> +        FID = ExpSemiLoc.first;
> +        LineNo = SrcMgr.getLineNumber(FID, ExpSemiLoc.second);
>       }
> -      if (!InAsmComment && InBraces && Tok.is(tok::r_brace) &&
> -          BraceCount == (savedBraceCount + 1)) {
> -        // Consume the closing brace, and finish
> -        EndLoc = ConsumeBrace();
> +    } else if (!InBraces || 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)
> +          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;
>       }
> -
> -      // 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 {
> -        AsmToks.push_back(Tok);
> -        ConsumeAnyToken();
> -      }
> -      TokLoc = Tok.getLocation();
> -      ++NumTokensRead;
> -    } while (1);
> -
> -    LineEnds.push_back(AsmToks.size());
> -
> -    if (InBraces && BraceCount != savedBraceCount) {
> -      // __asm without closing brace (this can happen at EOF).
> -      Diag(Tok, diag::err_expected_rbrace);
> -      Diag(LBraceLoc, diag::note_matching) << "{";
> -      return StmtError();
> -    } else if (NumTokensRead == 0) {
> -      // Empty __asm.
> -      Diag(Tok, diag::err_expected_lbrace);
> -      return StmtError();
>     }
> -    // Multiple adjacent asm's form together into a single asm statement
> -    // in the AST.
> -    if (!Tok.is(tok::kw_asm))
> +    if (!InAsmComment && InBraces && Tok.is(tok::r_brace) &&
> +        BraceCount == (savedBraceCount + 1)) {
> +      // Consume the closing brace, and finish
> +      EndLoc = ConsumeBrace();
>       break;
> -    EndLoc = ConsumeToken();
> +    }
> +
> +    // 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 {
> +      AsmToks.push_back(Tok);
> +      ConsumeAnyToken();
> +    }
> +    TokLoc = Tok.getLocation();
> +    ++NumTokensRead;
>   } while (1);
> 
> +  LineEnds.push_back(AsmToks.size());
> +
> +  if (InBraces && BraceCount != savedBraceCount) {
> +    // __asm without closing brace (this can happen at EOF).
> +    Diag(Tok, diag::err_expected_rbrace);
> +    Diag(LBraceLoc, diag::note_matching) << "{";
> +    return StmtError();
> +  } else if (NumTokensRead == 0) {
> +    // Empty __asm.
> +    Diag(Tok, diag::err_expected_lbrace);
> +    return StmtError();
> +  }
> +
>   // FIXME: We should be passing source locations for better diagnostics.
>   return Actions.ActOnMSAsmStmt(AsmLoc, llvm::makeArrayRef(AsmToks),
>                                 llvm::makeArrayRef(LineEnds), EndLoc);
> 
> Modified: cfe/trunk/test/CodeGen/ms-inline-asm.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-inline-asm.c?rev=161890&r1=161889&r2=161890&view=diff
> ==============================================================================
> --- cfe/trunk/test/CodeGen/ms-inline-asm.c (original)
> +++ cfe/trunk/test/CodeGen/ms-inline-asm.c Tue Aug 14 14:22:06 2012
> @@ -9,7 +9,9 @@
> 
> void t2() {
> // CHECK: @t2
> -// CHECK: call void asm sideeffect "nop\0Anop\0Anop", "~{dirflag},~{fpsr},~{flags}"() nounwind ia_nsdialect
> +// CHECK: call void asm sideeffect "nop", "~{dirflag},~{fpsr},~{flags}"() nounwind ia_nsdialect
> +// CHECK: call void asm sideeffect "nop", "~{dirflag},~{fpsr},~{flags}"() nounwind ia_nsdialect
> +// CHECK: call void asm sideeffect "nop", "~{dirflag},~{fpsr},~{flags}"() nounwind ia_nsdialect
> // CHECK: ret void
>   __asm nop
>   __asm nop
> @@ -18,14 +20,17 @@
> 
> void t3() {
> // CHECK: @t3
> -// CHECK: call void asm sideeffect "nop\0Anop\0Anop", "~{dirflag},~{fpsr},~{flags}"() nounwind ia_nsdialect
> +// CHECK: call void asm sideeffect "nop", "~{dirflag},~{fpsr},~{flags}"() nounwind ia_nsdialect
> +// CHECK: call void asm sideeffect "nop", "~{dirflag},~{fpsr},~{flags}"() nounwind ia_nsdialect
> +// CHECK: call void asm sideeffect "nop", "~{dirflag},~{fpsr},~{flags}"() nounwind ia_nsdialect
> // CHECK: ret void
>   __asm nop __asm nop __asm nop
> }
> 
> void t4(void) {
> // CHECK: @t4
> -// CHECK: call void asm sideeffect "mov ebx, eax\0Amov ecx, ebx", "~{dirflag},~{fpsr},~{flags}"() nounwind ia_nsdialect
> +// CHECK: call void asm sideeffect "mov ebx, eax", "~{dirflag},~{fpsr},~{flags}"() nounwind ia_nsdialect
> +// CHECK: call void asm sideeffect "mov ecx, ebx", "~{dirflag},~{fpsr},~{flags}"() nounwind ia_nsdialect
> // CHECK: ret void
>   __asm mov ebx, eax
>   __asm mov ecx, ebx
> @@ -33,8 +38,8 @@
> 
> void t5(void) {
> // CHECK: @t5
> -// CHECK: call void asm sideeffect "mov ebx, eax\0Amov ecx, ebx", "~{dirflag},~{fpsr},~{flags}"() nounwind ia_nsdialect
> +// CHECK: call void asm sideeffect "mov ebx, eax", "~{dirflag},~{fpsr},~{flags}"() nounwind ia_nsdialect
> +// CHECK: call void asm sideeffect "mov ecx, ebx", "~{dirflag},~{fpsr},~{flags}"() nounwind ia_nsdialect
> // CHECK: ret void
>   __asm mov ebx, eax __asm mov ecx, ebx
> }
> -
> 
> Modified: cfe/trunk/test/Parser/ms-inline-asm.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/ms-inline-asm.c?rev=161890&r1=161889&r2=161890&view=diff
> ==============================================================================
> --- cfe/trunk/test/Parser/ms-inline-asm.c (original)
> +++ cfe/trunk/test/Parser/ms-inline-asm.c Tue Aug 14 14:22:06 2012
> @@ -11,13 +11,13 @@
>   __asm { // expected-warning {{MS-style inline assembly is not supported}}
>     int 0x2c ; } asm comments are fun! }{
>   }
> -  __asm {}  // no warning as this gets merged with the previous inline asm
> +  __asm {} // expected-warning {{MS-style inline assembly is not supported}}
> }
> int t6() {
>   __asm int 3 ; } comments for single-line asm // expected-warning {{MS-style inline assembly is not supported}}
> -  __asm {} // no warning as this gets merged with the previous inline asm
> +  __asm {} // expected-warning {{MS-style inline assembly is not supported}}
> 
> -  __asm int 4 // no warning as this gets merged with the previous inline asm
> +  __asm int 4 // expected-warning {{MS-style inline assembly is not supported}}
>   return 10;
> }
> int t7() {
> @@ -28,10 +28,10 @@
>   }
> }
> void t8() {
> -  __asm nop __asm nop __asm nop // expected-warning {{MS-style inline assembly is not supported}}
> +  __asm nop __asm nop __asm nop // expected-warning {{MS-style inline assembly is not supported}} expected-warning {{MS-style inline assembly is not supported}} expected-warning {{MS-style inline assembly is not supported}}
> }
> void t9() {
> -  __asm nop __asm nop ; __asm nop // expected-warning {{MS-style inline assembly is not supported}}
> +  __asm nop __asm nop ; __asm nop // expected-warning {{MS-style inline assembly is not supported}} expected-warning {{MS-style inline assembly is not supported}}
> }
> int t_fail() { // expected-note {{to match this}}
>   __asm
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits




More information about the cfe-commits mailing list