[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 ↗

(On Diff #49917)

Can this code be moved into parseStatement()?

[myatsina]

lib/MC/MCParser/AsmParser.cpp

4989 ↗

(On Diff #49917)

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 ↗

(On Diff #49917)

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

1826 ↗

(On Diff #49946)

line alignment

1838 ↗

(On Diff #49946)

line alignment

5006 ↗

(On Diff #49946)

remove {}

lib/Target/X86/AsmParser/X86AsmParser.cpp

2307 ↗

(On Diff #49946)

auto isCurlyBrace = []() {

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

}

2340 ↗

(On Diff #49946)

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

2349 ↗

(On Diff #49946)

else if (isCurlyBrace())

2350 ↗

(On Diff #49946)

brace

[myatsina]

lib/Target/X86/AsmParser/X86AsmParser.cpp

2307 ↗

(On Diff #49946)

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

llvm/trunk/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;

llvm/trunk/lib/MC/MCParser/AsmParser.cpp

public:
 
   void checkForValidSection() override;
   /// }
 
 private:
 
   bool parseStatement(ParseStatementInfo &Info,
                       MCAsmParserSemaCallback *SI);
+  bool parseCurlyBlockScope(SmallVectorImpl<AsmRewrite>& AsmStrRewrites);
   void eatToEndOfLine();
   bool parseCppHashLineFilenameComment(SMLoc L);
 
   void checkForBadMacro(SMLoc DirectiveLoc, StringRef Name, StringRef Body,
                         ArrayRef<MCAsmMacroParameter> Parameters);
   bool expandMacro(raw_svector_ostream &OS, StringRef Body,
                    ArrayRef<MCAsmMacroParameter> Parameters,
                    ArrayRef<MCAsmMacroArgument> A, bool EnableAtPseudoVariable,
getTargetParser().MatchAndEmitInstruction(IDLoc, Info.Opcode,
                                               ErrorInfo, ParsingInlineAsm);
   }
 
   // Don't skip the rest of the line, the instruction parser is responsible for
   // that.
   return false;
 }
 
+// Parse and erase curly braces marking block start/end
+bool 
+AsmParser::parseCurlyBlockScope(SmallVectorImpl<AsmRewrite> &AsmStrRewrites) {
+  // Identify curly brace marking block start/end
+  if (Lexer.isNot(AsmToken::LCurly) && Lexer.isNot(AsmToken::RCurly))
+    return false;
+
+  SMLoc StartLoc = Lexer.getLoc();
+  Lex(); // Eat the brace
+  if (Lexer.is(AsmToken::EndOfStatement))
+    Lex(); // Eat EndOfStatement following the brace
+
+  // Erase the block start/end brace from the output asm string
+  AsmStrRewrites.emplace_back(AOK_Skip, StartLoc, Lexer.getLoc().getPointer() -
+                                                  StartLoc.getPointer());
+  return true;
+}
+
 /// 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();
 }
bool AsmParser::parseMSInlineAsm(
 
   // Prime the lexer.
   Lex();
 
   // While we have input, parse each statement.
   unsigned InputIdx = 0;
   unsigned OutputIdx = 0;
   while (getLexer().isNot(AsmToken::Eof)) {
+    // Parse curly braces marking block start/end
+    if (parseCurlyBlockScope(AsmStrRewrites))
+      continue;
+
     ParseStatementInfo Info(&AsmStrRewrites);
     if (parseStatement(Info, &SI))
       return true;
 
     if (Info.ParseError)
       return true;
 
     if (Info.Opcode == ~0U)
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)

llvm/trunk/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)