[ms-inline-asm][AVX512] Add ability to use k registers in MS inline asm + fix bag with curly braces

[myatsina]

Summary

Until now curly braces could only be used in MS inline assembly to mark block start/end.
All curly braces were removed completely at a very early stage.
This approach caused bugs like:
"m{o}v eax, ebx" turned into "mov eax, ebx" without any error.

In addition, AVX-512 added special operands (e.g., k registers), which are also surrounded by curly braces that mark them as such.
Now, we need to keep the curly braces and identify at a later stage if they are marking block start/end (if so, ignore them), or surrounding special AVX-512 operands (if so, parse them as such).

This patch fixes the bug described above and enables the use of AVX-512 special operands.

This review is for the llvm part.
The tests and clang part of the review is: http://reviews.llvm.org/D17766

[myatsina]

ping
the clang part of the review was approved, I'm still waiting for approval on the llvm part

[• ehsan]

This looks good to me, but I would prefer someone else to also look over both parts...

[myatsina]

Simplified the code of the llvm part of the patch

[delena]

lib/MC/MCParser/AsmParser.cpp

4989

Can this code be moved into parseStatement()?

[myatsina]

lib/MC/MCParser/AsmParser.cpp

4989

If we this code to parseStatement() then .s files that contain blocks with braces will be legal.
I don't think we want llvm-mc to be able to accept such syntax.

[delena]

lib/MC/MCParser/AsmParser.cpp

5005

Let's take it to a separate function and remove LastStatementLoc.

[myatsina]

Applied requested modifications

[myatsina]

In D17767#368846, @myatsina wrote:

Applied requested modifications

A test in the clang part of the review was also changed to reflect this changes (review of the clang-part: http://reviews.llvm.org/D17766)

[delena]

lib/MC/MCParser/AsmParser.cpp

1825

line alignment

1837

line alignment

4988

remove {}

lib/Target/X86/AsmParser/X86AsmParser.cpp

2307

auto isCurlyBrace = []() {

return getLexer().is(AsmToken::LCurly) || getLexer().is(AsmToken::RCurly);

}

2340

if (getLexer().isNot(AsmToken::EndOfStatement) && !isCurlyBrace())

2349

else if (isCurlyBrace())

2350

brace

[myatsina]

lib/Target/X86/AsmParser/X86AsmParser.cpp

2307

We do need the logic that checks that we're dealing with intel syntax and parsing inline asm (otherwise we're parsing asm code of an s file).
I prefer this logic as a variable and not a function.

[myatsina]

Fixed formatting

[delena]

LGTM

include/llvm/MC/MCParser/MCTargetAsmParser.h

AOK_DotOperator,    // Rewrite a dot operator expression as an immediate.
                       // E.g., [eax].foo.bar -> [eax].8
   AOK_Emit,           // Rewrite _emit as .byte.
   AOK_Imm,            // Rewrite as $$N.
   AOK_ImmPrefix,      // Add $$ before a parsed Imm.
   AOK_Input,          // Rewrite in terms of $N.
   AOK_Output,         // Rewrite in terms of $N.
   AOK_SizeDirective,  // Add a sizing directive (e.g., dword ptr).
   AOK_Label,          // Rewrite local labels.
+  AOK_EndOfStatement, // Add EndOfStatement (e.g., "\n\t").
   AOK_Skip            // Skip emission (e.g., offset/type operators).
 };
 
 const char AsmRewritePrecedence [] = {
   0, // AOK_Delete
   2, // AOK_Align
   2, // AOK_EVEN
   2, // AOK_DotOperator
   2, // AOK_Emit
   4, // AOK_Imm
   4, // AOK_ImmPrefix
   3, // AOK_Input
   3, // AOK_Output
   5, // AOK_SizeDirective
   1, // AOK_Label
+  5, // AOK_EndOfStatement
   2  // AOK_Skip
 };
 
 struct AsmRewrite {
   AsmRewriteKind Kind;
   SMLoc Loc;
   unsigned Len;
   unsigned Val;

lib/MC/MCParser/AsmParser.cpp

bool AsmParser::parseStatement(ParseStatementInfo &Info,
 
   // Don't skip the rest of the line, the instruction parser is responsible for
   // that.
   return false;
 }
 
 /// eatToEndOfLine uses the Lexer to eat the characters to the end of the line
 /// since they may not be able to be tokenized to get to the end of line token.
 void AsmParser::eatToEndOfLine() {
   if (!Lexer.is(AsmToken::EndOfStatement))
     Lexer.LexUntilEndOfLine();
   // Eat EOL.
   Lex();
 }
 
 /// parseCppHashLineFilenameComment as this:
 ///   ::= # number "filename"
 /// or just as a full line comment if it doesn't have a number and a string.
 bool AsmParser::parseCppHashLineFilenameComment(SMLoc L) {
   Lex(); // Eat the hash token.
 
   if (getLexer().isNot(AsmToken::Integer)) {
     // Consume the line since in cases it is not a well-formed line directive,
     // as if were simply a full line comment.
     eatToEndOfLine();
     return false;
   }
 
   int64_t LineNumber = getTok().getIntVal();
bool AsmParser::parseMSInlineAsm(
   SmallVector<AsmRewrite, 4> AsmStrRewrites;
 
   // Prime the lexer.
   Lex();
 
   // While we have input, parse each statement.
   unsigned InputIdx = 0;
   unsigned OutputIdx = 0;
+  SMLoc LastStatementLoc = getLexer().getLoc();
   while (getLexer().isNot(AsmToken::Eof)) {
     ParseStatementInfo Info(&AsmStrRewrites);
+
+    if (Lexer.is(AsmToken::LCurly) || Lexer.is(AsmToken::RCurly)) {
+      // Identify curly brace marking block start/end
+      SMLoc StartLoc = Lexer.getLoc();
+      Lex(); // Eat curly brace
+      if (Lexer.is(AsmToken::EndOfStatement))
+        Lex(); // Eat EndOfStatement after curly brace
+
+      // Erase the block start/end brace from the output asm string
+      AsmStrRewrites.emplace_back(AOK_Skip, StartLoc,
+        Lexer.getLoc().getPointer() -
+        StartLoc.getPointer());
+      continue;
+    }
+
     if (parseStatement(Info, &SI))
       return true;
+    LastStatementLoc = Lexer.getLoc();
 
     if (Info.ParseError)
       return true;
 
     if (Info.Opcode == ~0U)
       continue;
 
     const MCInstrDesc &Desc = MII->get(Info.Opcode);
while (getLexer().isNot(AsmToken::Eof)) {
     }
 
     // Consider implicit defs to be clobbers.  Think of cpuid and push.
     ArrayRef<MCPhysReg> ImpDefs(Desc.getImplicitDefs(),
                                 Desc.getNumImplicitDefs());
     ClobberRegs.insert(ClobberRegs.end(), ImpDefs.begin(), ImpDefs.end());
   }
 
+  // We don't want to add another new line before EOF
+  for (AsmRewrite &AR : AsmStrRewrites) {
+    if (AR.Loc.getPointer() < LastStatementLoc.getPointer())
+      continue;
+    if (AR.Kind == AOK_EndOfStatement)
+      AR.Kind = AOK_Delete;
+  }
+
   // Set the number of Outputs and Inputs.
   NumOutputs = OutputDecls.size();
   NumInputs = InputDecls.size();
 
   // Set the unique clobbers.
   array_pod_sort(ClobberRegs.begin(), ClobberRegs.end());
   ClobberRegs.erase(std::unique(ClobberRegs.begin(), ClobberRegs.end()),
                     ClobberRegs.end());
case AOK_EVEN:
       break;
     case AOK_DotOperator:
       // Insert the dot if the user omitted it.
       OS.flush();
       if (AsmStringIR.back() != '.')
         OS << '.';
       OS << AR.Val;
       break;
+    case AOK_EndOfStatement:
+      OS << "\n\t";
+      break;
     }
 
     // Skip the original expression.
     AsmStart = Loc + AR.Len + AdditionalSkip;
   }
 
   // Emit the remainder of the asm string.
   if (AsmStart != AsmEnd)

lib/Target/X86/AsmParser/X86AsmParser.cpp

bool X86AsmParser::ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
 
   // Determine whether this is an instruction prefix.
   bool isPrefix =
     Name == "lock" || Name == "rep" ||
     Name == "repe" || Name == "repz" ||
     Name == "repne" || Name == "repnz" ||
     Name == "rex64" || Name == "data16";
 
+  bool CurlyAsEndOfStatement = false;
   // This does the actual operand parsing.  Don't parse any more if we have a
   // prefix juxtaposed with an operation like "lock incl 4(%rax)", because we
   // just want to parse the "lock" as the first instruction and the "incl" as
   // the next one.
   if (getLexer().isNot(AsmToken::EndOfStatement) && !isPrefix) {
 
     // Parse '*' modifier.
     if (getLexer().is(AsmToken::Star))
while(1) {
       }
       // check for comma and eat it
       if (getLexer().is(AsmToken::Comma))
         Parser.Lex();
       else
         break;
      }
 
-    if (getLexer().isNot(AsmToken::EndOfStatement))
+    // In MS inline asm curly braces mark the begining/end of a block, therefore
+    // they should be interepreted as end of statement
+    CurlyAsEndOfStatement =
+        isParsingIntelSyntax() && isParsingInlineAsm() &&
+        (getLexer().is(AsmToken::LCurly) || getLexer().is(AsmToken::RCurly));
+    if (getLexer().isNot(AsmToken::EndOfStatement) && !CurlyAsEndOfStatement)
       return ErrorAndEatStatement(getLexer().getLoc(),
                                   "unexpected token in argument list");
    }
 
   // Consume the EndOfStatement or the prefix separator Slash
   if (getLexer().is(AsmToken::EndOfStatement) ||
       (isPrefix && getLexer().is(AsmToken::Slash)))
     Parser.Lex();
+  else if (CurlyAsEndOfStatement)
+    // Add an actual EndOfStatement before the curly brace
+    Info.AsmRewrites->emplace_back(AOK_EndOfStatement,
+                                   getLexer().getTok().getLoc(), 0);
 
   // This is for gas compatibility and cannot be done in td.
   // Adding "p" for some floating point with no argument.
   // For example: fsub --> fsubp
   bool IsFp =
     Name == "fsub" || Name == "fdiv" || Name == "fsubr" || Name == "fdivr";
   if (IsFp && Operands.size() == 1) {
     const char *Repl = StringSwitch<const char *>(Name)