[X86] Add support for vex, vex2, vex3, and evex for MASM

[LiuChen3]

Summary

For MASM syntax, the prefixes are not enclosed in braces.
The assembly code should like:

"evex vcvtps2pd xmm0, xmm1"

There are still some avx512 tests need to be improved with 'evex'
prefix. But I think it's better to discuss this syntax first.

[pengfei]

clang/test/CodeGen/X86/ms-inline-asm-prefix.c

2

Maybe need // REQUIRES: x86-registered-target

2

You may need add att check too since you modified the att code.

2

Should it be avalible only when -fms-compatibility

[epastor]

clang/test/CodeGen/X86/ms-inline-asm-prefix.c

2

The triple is misspelled; it should be x86_64-pc-windows-msvc (the "n" in windows is missing)

2

A broader question: As written, this applies to anything in Intel syntax. Is this an Intel syntax feature, or a MASM feature?

[LiuChen3]

clang/test/CodeGen/X86/ms-inline-asm-prefix.c

2

Thanks for your review. After checking with the people of MSVC, I found that prefix without braces is not intel syntax. Actually, we don't know if there any document says what the prefix should be. At least, gcc does have the "{}" in intel syntax, so does clang. We currently decide to only support parsing the prefix without MSVC.

[LiuChen3]

1. Address comments;
2. Only support parsing vex/vex2/vex3/evex prefix for MASM

[LiuChen3]

clang/test/CodeGen/X86/ms-inline-asm-prefix.c

2

I am not sure. But I think -fasm-blocks, -fms-extensions also support MASM.

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

3068

I tried to limit to MASM. But I found that the 'isParsingMSInlineAsm()' is not accurate. And then I tried to transmit 'ParsingMSInlineAsm' information correctly in AsmPrinterInlineAsm.cpp (according to the '-fasm-blocks' option). But I was surprised to find that isParsingMSInlineAsm() is actually used as the argument of 'MatchingInlineAsm' in 'MatchAndEmitInstruction()'. This makes me confused. Should that 'MatchingInlineAsm' be 'MatchingMSInlineAsm' ?Is this MatchingInlineAsm only used by llvm-ml.
It difficult to limit this to MASM at the moment.

[epastor]

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

3068

llvm-ml attempts not to touch anything involving inline assembly so far. The signal that MasmParser.cpp is involved is Parser.isParsingMasm(). However... while I can't answer the majority of this without more research, I suspect you're correct that MatchingInlineAsm is misnamed. We need to check this, and if so, we should rename it to avoid confusion.

[craig.topper]

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

3068

MS inline assembly is parsed twice. Once by clang to find names of C/C++ variables. And again in the backend. GNU inline assembly is only parsed in the backend since variable names are bound explicitly and not referenced in the assembly text.

IsParsingInlineAsm is set during the clang parsing.

[LiuChen3]

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

3068

Thanks. That's make sense.
So 'MatchingInlineAsm' in MatchAndEmitInstruction() more like 'MatchingMSInlineAsm' and can only be set when parsing the instructions first time. And the second time parser can not set 'setParsingMSInlineAsm(true)'. That's make difficult to limit the scope to MASM for now.

[LiuChen3]

Rebase.
Adding the '{}' to prefix when generate IR.

[LiuChen3]

clang/lib/AST/Stmt.cpp

801

From X86AsmParser, the vex/evex prefix must be the begin of the instructions.

[LiuChen3]

Ping?

[pengfei]

clang/lib/AST/Stmt.cpp

795

Can we always assume the separator is \n\t?

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

3083

You just need to check ForcedVEXEncoding != VEXEncoding_Default.

3084

Unused assignment. It may suppose to be used on line 3086.

[LiuChen3]

clang/lib/AST/Stmt.cpp

795

I think so. From the code, we can see '\n\t' will be added to each end of statement:

case AOK_EndOfStatement:
      OS << "\n\t";
      break;

[LiuChen3]

1. Check prefix, ignoring case
2. Delete IsPrefix parameter, and delete 'break', so that we won't check prefix again. I am not sure if this is right. Att format can allow two prefix and using the last one as the finally encoding prefix. I think this may not be the original intention of the design.
3. Change the test: checking the IR istead of checking the assembly.
4. Made some format adjustments.

[LiuChen3]

2. Delete IsPrefix parameter, and delete 'break'

It should be 'continue'. Sorry for this mistake.

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

3083

I think this is better. Multi vex/evex prefix doesn't make sense.

3084

This would be used later. However, this should only be updated when there is prefix. I put it in wrong place.

[pengfei]

2. Delete IsPrefix parameter, and delete 'break', so that we won't check prefix again. I am not sure if this is right. Att format can allow two prefix and using the last one as the finally encoding prefix. I think this may not be the original intention of the design.

It allows more than two, right? like {vex}{vex2}{vex3} instruction. I think it should be a bug for att.

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

3069

Do you need to eat the prefix here?

[LiuChen3]

It allows more than two, right? like {vex}{vex2}{vex3} instruction. I think it should be a bug for att.

Yes, My previous statement is incorrect, it should be ‘two more’. Thanks for your correction.
We might need another patch to fix it.

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

3069

No. The prefix has been eat.
For example: vex vcvtps2pd xmm0, xmm1 .
Current token is 'vex' and the rest is 'vcvtps2pd xmm0, xmm1'. 'vcvtps2pd' is the next token which will be eat in line 3082.

[pengfei]

LGTM. Thanks.
You'd better wait one or two days to see if other people objects.

clang/lib/AST/Stmt.cpp

else
                    Piece.getModifier() + '}';
   }
   return AsmString;
 }
 
 /// Assemble final IR asm string (MS-style).
 std::string MSAsmStmt::generateAsmString(const ASTContext &C) const {
   // FIXME: This needs to be translated into the IR string representation.
-  return std::string(AsmStr);
+  SmallVector<StringRef, 8> Pieces;
+  AsmStr.split(Pieces, "\n\t");
+  std::string MSAsmString;
+  for (size_t i = 0, e = Pieces.size(); i < e; ++i) {
+    StringRef Instruction = Pieces[i];
+    // For vex/vex2/vex3/evex masm style prefix, convert it to att style
+    // since we don't support masm style prefix in backend.
+    if (Instruction.startswith("vex "))
+      MSAsmString += '{' + Instruction.substr(0, 3).str() + '}' +
+                     Instruction.substr(3).str();
+    else if (Instruction.startswith("vex2 ") ||
+             Instruction.startswith("vex3 ") || Instruction.startswith("evex "))
+      MSAsmString += '{' + Instruction.substr(0, 4).str() + '}' +
+                     Instruction.substr(4).str();
+    else
+      MSAsmString += Instruction.str();
+    // If this is not the last instruction, adding back the '\n\t'.
+    if (i < e - 1)
+      MSAsmString += "\n\t";
+  }
+  return MSAsmString;
 }
 
 Expr *MSAsmStmt::getOutputExpr(unsigned i) {
   return cast<Expr>(Exprs[i]);
 }
 
 Expr *MSAsmStmt::getInputExpr(unsigned i) {
   return cast<Expr>(Exprs[i + NumOutputs]);

clang/test/CodeGen/X86/ms-inline-asm-prefix.c

+// REQUIRES: x86-registered-target
+// RUN:%clang_cc1 %s -ferror-limit 0 -triple=x86_64-pc-windows-msvc -target-feature +avx512f -target-feature +avx2 -target-feature +avx512vl -fasm-blocks -mllvm -x86-asm-syntax=intel -S -o -  | FileCheck %s -check-prefix=INTEL
+// RUN:%clang_cc1 %s -ferror-limit 0 -triple=x86_64-pc-windows-msvc -target-feature +avx512f -target-feature +avx2 -target-feature +avx512vl -fasm-blocks -mllvm -x86-asm-syntax=att -S -o -  | FileCheck %s -check-prefix=ATT
+
+void check_inline_prefix(void) {
+  __asm {
+    // INTEL: {vex}     vcvtps2pd       xmm0, xmm1
+    // INTEL: {vex2}    vcvtps2pd       xmm0, xmm1
+    // INTEL: {vex3}    vcvtps2pd       xmm0, xmm1
+    // INTEL: {evex}    vcvtps2pd       xmm0, xmm1
+    // ATT:   {vex}   vcvtps2pd       %xmm1, %xmm0
+    // ATT:   {vex2}  vcvtps2pd       %xmm1, %xmm0
+    // ATT:   {vex3}  vcvtps2pd       %xmm1, %xmm0
+    // ATT:   {evex}  vcvtps2pd       %xmm1, %xmm0
+    vex vcvtps2pd xmm0, xmm1
+    vex2 vcvtps2pd xmm0, xmm1
+    vex3 vcvtps2pd xmm0, xmm1
+    evex vcvtps2pd xmm0, xmm1
+  }
+}

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

if (Name == "{") {
         if (getLexer().isNot(AsmToken::Identifier))
           return Error(Parser.getTok().getLoc(), "Expected identifier");
         // FIXME: The mnemonic won't match correctly if its not in lower case.
         Name = Parser.getTok().getString();
         Parser.Lex();
       }
       continue;
     }
-
+    // Parse MASM style pseudo prefixes.
+    if (isParsingMSInlineAsm()) {
+      bool IsPrefix = false;
+      if (Name == "vex") {
+        ForcedVEXEncoding = VEXEncoding_VEX;
+        IsPrefix = true;
+      } else if (Name == "vex2") {
+        ForcedVEXEncoding = VEXEncoding_VEX2;
+        IsPrefix = true;
+      } else if (Name == "vex3") {
+        ForcedVEXEncoding = VEXEncoding_VEX3;
+        IsPrefix = true;
+      } else if (Name == "evex") {
+        ForcedVEXEncoding = VEXEncoding_EVEX;
+        IsPrefix = true;
+      }
+      if (IsPrefix) {
+        NameLoc = Parser.getTok().getLoc();
+        if (getLexer().isNot(AsmToken::Identifier))
+          return Error(Parser.getTok().getLoc(), "Expected identifier");
+        // FIXME: The mnemonic won't match correctly if its not in lower case.
+        Name = Parser.getTok().getString();
+        Parser.Lex();
+        continue;
+      }
+    }
     break;
   }
 
   // Support the suffix syntax for overriding displacement size as well.
   if (Name.consume_back(".d32")) {
     ForcedDispEncoding = DispEncoding_Disp32;
   } else if (Name.consume_back(".d8")) {
     ForcedDispEncoding = DispEncoding_Disp8;
bool X86AsmParser::MatchAndEmitIntelInstruction(SMLoc IDLoc, unsigned &Opcode,
   unsigned Prefixes = getPrefixes(Operands);
 
   // First, handle aliases that expand to multiple instructions.
   MatchFPUWaitAlias(IDLoc, static_cast<X86Operand &>(*Operands[0]), Operands, Out, MatchingInlineAsm);
   X86Operand &Op = static_cast<X86Operand &>(*Operands[0]);
 
   MCInst Inst;
 
-  // If VEX3 encoding is forced, we need to pass the USE_VEX3 flag to the
-  // encoder.
-  if (ForcedVEXEncoding == VEXEncoding_VEX3)
+  // If VEX/EVEX encoding is forced, we need to pass the USE_* flag to the
+  // encoder and printer.
+  if (ForcedVEXEncoding == VEXEncoding_VEX)
+    Prefixes |= X86::IP_USE_VEX;
+  else if (ForcedVEXEncoding == VEXEncoding_VEX2)
+    Prefixes |= X86::IP_USE_VEX2;
+  else if (ForcedVEXEncoding == VEXEncoding_VEX3)
     Prefixes |= X86::IP_USE_VEX3;
+  else if (ForcedVEXEncoding == VEXEncoding_EVEX)
+    Prefixes |= X86::IP_USE_EVEX;
 
   // Set encoded flags for {disp8} and {disp32}.
   if (ForcedDispEncoding == DispEncoding_Disp8)
     Prefixes |= X86::IP_USE_DISP8;
   else if (ForcedDispEncoding == DispEncoding_Disp32)
     Prefixes |= X86::IP_USE_DISP32;
 
   if (Prefixes)