[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

1

Maybe need // REQUIRES: x86-registered-target

1

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

1

Should it be avalible only when -fms-compatibility

[epastor]

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

1

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

1

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

1

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

1

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

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

2851

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

2851

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

2851

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

2851

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 ↗

(On Diff #304730)

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

[LiuChen3]

Ping?

[pengfei]

clang/lib/AST/Stmt.cpp

795 ↗

(On Diff #304730)

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

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

2866

You just need to check ForcedVEXEncoding != VEXEncoding_Default.

2867

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

[LiuChen3]

clang/lib/AST/Stmt.cpp

795 ↗

(On Diff #304730)

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

2866

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

2867

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

2852

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

2852

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/test/CodeGen/X86/ms-inline-asm-prefix.c

+// RUN:%clang_cc1 %s -ferror-limit 0 -triple=x86_64-pc-widows-msvc -target-feature +avx512f -target-feature +avx2 -target-feature +avx512vl -fasm-blocks -mllvm -x86-asm-syntax=intel -S -o -  | FileCheck %s -check-prefix CHECK
+
+void check_inline_prefix(void) {
+  __asm {
+// CHECK: vex     vcvtps2pd       xmm0, xmm1
+// CHECK: vex     vcvtps2pd       xmm0, xmm1
+// CHECK: vex3    vcvtps2pd       xmm0, xmm1
+// CHECK: evex    vcvtps2pd       xmm0, xmm1
+    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 (isParsingIntelSyntax()) {
+      bool IsPrefix = false;
+      if (Name == "vex" || Name == "vex2") {
+        ForcedVEXEncoding = VEXEncoding_VEX;
+        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();
+      }
+    }
     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)

llvm/lib/Target/X86/MCTargetDesc/X86ATTInstPrinter.cpp

void X86ATTInstPrinter::printInst(const MCInst *MI, uint64_t Address,
                                   StringRef Annot, const MCSubtargetInfo &STI,
                                   raw_ostream &OS) {
   // If verbose assembly is enabled, we can print some informative comments.
   if (CommentStream)
     HasCustomInstComment = EmitAnyX86InstComments(MI, *CommentStream, MII);
 
   printInstFlags(MI, OS);
 
+  unsigned Flags = MI->getFlags();
+  // These all require a pseudo prefix
+  if (Flags & X86::IP_USE_VEX)
+    OS << "\t{vex}";
+  else if (Flags & X86::IP_USE_VEX2)
+    OS << "\t{vex2}";
+  else if (Flags & X86::IP_USE_VEX3)
+    OS << "\t{vex3}";
+  else if (Flags & X86::IP_USE_EVEX)
+    OS << "\t{evex}";
+
+  if (Flags & X86::IP_USE_DISP8)
+    OS << "\t{disp8}";
+  else if (Flags & X86::IP_USE_DISP32)
+    OS << "\t{disp32}";
+
   // Output CALLpcrel32 as "callq" in 64-bit mode.
   // In Intel annotation it's always emitted as "call".
   //
   // TODO: Probably this hack should be redesigned via InstAlias in
   // InstrInfo.td as soon as Requires clause is supported properly
   // for InstAlias.
   if (MI->getOpcode() == X86::CALLpcrel32 &&
       (STI.getFeatureBits()[X86::Mode64Bit])) {

llvm/lib/Target/X86/MCTargetDesc/X86InstPrinterCommon.cpp

void X86InstPrinterCommon::printInstFlags(const MCInst *MI, raw_ostream &O) {
 
   if ((TSFlags & X86II::NOTRACK) || (Flags & X86::IP_HAS_NOTRACK))
     O << "\tnotrack\t";
 
   if (Flags & X86::IP_HAS_REPEAT_NE)
     O << "\trepne\t";
   else if (Flags & X86::IP_HAS_REPEAT)
     O << "\trep\t";
-
-  // These all require a pseudo prefix
-  if (Flags & X86::IP_USE_VEX)
-    O << "\t{vex}";
-  else if (Flags & X86::IP_USE_VEX2)
-    O << "\t{vex2}";
-  else if (Flags & X86::IP_USE_VEX3)
-    O << "\t{vex3}";
-  else if (Flags & X86::IP_USE_EVEX)
-    O << "\t{evex}";
-
-  if (Flags & X86::IP_USE_DISP8)
-    O << "\t{disp8}";
-  else if (Flags & X86::IP_USE_DISP32)
-    O << "\t{disp32}";
 }
 
 void X86InstPrinterCommon::printVKPair(const MCInst *MI, unsigned OpNo,
                                        raw_ostream &OS) {
   // In assembly listings, a pair is represented by one of its members, any
   // of the two.  Here, we pick k0, k2, k4, k6, but we could as well
   // print K2_K3 as "k3".  It would probably make a lot more sense, if
   // the assembly would look something like:

llvm/lib/Target/X86/MCTargetDesc/X86IntelInstPrinter.cpp

void X86IntelInstPrinter::printRegName(raw_ostream &OS, unsigned RegNo) const {
   OS << getRegisterName(RegNo);
 }
 
 void X86IntelInstPrinter::printInst(const MCInst *MI, uint64_t Address,
                                     StringRef Annot, const MCSubtargetInfo &STI,
                                     raw_ostream &OS) {
   printInstFlags(MI, OS);
 
+  unsigned Flags = MI->getFlags();
+  // These all require a pseudo prefix
+  if (Flags & X86::IP_USE_VEX)
+    OS << "\tvex";
+  else if (Flags & X86::IP_USE_VEX2)
+    OS << "\tvex2";
+  else if (Flags & X86::IP_USE_VEX3)
+    OS << "\tvex3";
+  else if (Flags & X86::IP_USE_EVEX)
+    OS << "\tevex";
+
   // In 16-bit mode, print data16 as data32.
   if (MI->getOpcode() == X86::DATA16_PREFIX &&
       STI.getFeatureBits()[X86::Mode16Bit]) {
     OS << "\tdata32";
   } else if (!printAliasInstr(MI, Address, OS) && !printVecCompareInstr(MI, OS))
     printInstruction(MI, Address, OS);
 
   // Next always print the annotation.