[X86] VEX/EVEX prefix doesn't work for inline assembly.

[LiuChen3]

Summary

For now, we lost the encoding information if we using inline assembly.
The encoding for the inline assembly will keep default even if we add
the vex/evex prefix.

[pengfei]

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

12

Better adding a no prefix one.

llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h

57–70

Can we make these = 1U << aligned?

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

352

"\t{vex}\t" ?

[pengfei]

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

2824

I think it's reasonable if we generate "{vex}" for input "{vex2}"

[LiuChen3]

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

2824

GCC will out put {vex2} if the input is {vex2}.

[craig.topper]

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

3896

Why do we need Force_VEX3Encoding and IP_USE_VEX3?

llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h

67

Why don't these start with IP_?

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

354

Is it important that we use {vex2} instead of just treating it as {vex}?

[craig.topper]

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

354

I guess we'd actually have to print {vex2} instead of {vex} for compatibility with older versions of GNU assember that don't support {vex}. So I guess maybe they should be different as much as I hate wasting flags

[craig.topper]

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

358

We also need to print {disp8} and {disp32} here to fix the same bug with those right?

[craig.topper]

Does this bug only effect the printing of inline assembly to a .s file? The encoder should work correctly even without this I think?

[LiuChen3]

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

14

Does this bug only effect the printing of inline assembly to a .s file?

Yes. Using "-c" to out .o file directly will get right encoding.

But if we firstly output the .s file and then compile it, the end encoding is wrong. For this example, it will all be two-byte vex prefix.

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

3896

I think this will make all of IP_USE_VEX3 the 3-byte vex prefix instruction output with {vex3}. The IP_USE_VEX3 is for encoder.

[craig.topper]

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

3896

Isn't this the only place we set IP_USE_VEX3?

[LiuChen3]

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

3896

Yes. I think it's reasonable to use this flag only. And naming other Force_* to IP_USE_* .

[LiuChen3]

Address comments

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

352

No "\t" needed to add to the end of prefix. The printer will handle it correctly.

[craig.topper]

LGTM

[kiranchandramohan]

This change probably requires the X86 target.
// REQUIRES: x86-registered-target

Builds which target AArch64 only have been failing due to this change.
http://lab.llvm.org:8011/#/builders/32/builds/291

[kiranchandramohan]

I have added a fix to run the test only when the X86 target is available. Please feel free to change if it is not the correct fix.
https://github.com/llvm/llvm-project/commit/c551ba0e90bd2b49ef501d591f8362ba44e5484d

[LiuChen3]

In D90009#2354849, @kiranchandramohan wrote:

I have added a fix to run the test only when the X86 target is available. Please feel free to change if it is not the correct fix.
https://github.com/llvm/llvm-project/commit/c551ba0e90bd2b49ef501d591f8362ba44e5484d

Thanks for your help!

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

+// RUN:%clang_cc1 %s -ferror-limit 0 -triple=x86_64-pc -target-feature +avx512f -target-feature +avx2 -target-feature +avx512vl -S -o -  | FileCheck %s -check-prefix CHECK
+
+// This test is to check if the prefix in inline assembly is correctly
+// preserved.
+
+void check_inline_prefix(void) {
+  __asm__ (
+// CHECK: {vex} vcvtps2pd %xmm0, %xmm1
+// CHECK: {vex2} vcvtps2pd %xmm0, %xmm1
+// CHECK: {vex3} vcvtps2pd %xmm0, %xmm1
+// CHECK: {evex} vcvtps2pd %xmm0, %xmm1
+    "{vex} vcvtps2pd %xmm0, %xmm1\n\t"
+    "{vex2} vcvtps2pd %xmm0, %xmm1\n\t"
+    "{vex3} vcvtps2pd %xmm0, %xmm1\n\t"
+    "{evex} vcvtps2pd %xmm0, %xmm1\n\t"
+  );
+}

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

 class X86AsmParser : public MCTargetAsmParser {
   ParseInstructionInfo *InstInfo;
   bool Code16GCC;
   unsigned ForcedDataPrefix = 0;
 
   enum VEXEncoding {
     VEXEncoding_Default,
     VEXEncoding_VEX,
+    VEXEncoding_VEX2,
     VEXEncoding_VEX3,
     VEXEncoding_EVEX,
   };
 
   VEXEncoding ForcedVEXEncoding = VEXEncoding_Default;
 
   enum DispEncoding {
     DispEncoding_Default,
if (Name == "{") {
       if (getLexer().isNot(AsmToken::Identifier))
         return Error(Parser.getTok().getLoc(), "Unexpected token after '{'");
       std::string Prefix = Parser.getTok().getString().lower();
       Parser.Lex(); // Eat identifier.
       if (getLexer().isNot(AsmToken::RCurly))
         return Error(Parser.getTok().getLoc(), "Expected '}'");
       Parser.Lex(); // Eat curly.
 
-      if (Prefix == "vex" || Prefix == "vex2")
+      if (Prefix == "vex")
         ForcedVEXEncoding = VEXEncoding_VEX;
+      else if (Prefix == "vex2")
+        ForcedVEXEncoding = VEXEncoding_VEX2;
       else if (Prefix == "vex3")
         ForcedVEXEncoding = VEXEncoding_VEX3;
       else if (Prefix == "evex")
         ForcedVEXEncoding = VEXEncoding_EVEX;
       else if (Prefix == "disp8")
         ForcedDispEncoding = DispEncoding_Disp8;
       else if (Prefix == "disp32")
         ForcedDispEncoding = DispEncoding_Disp32;
unsigned X86AsmParser::checkTargetMatchPredicate(MCInst &Inst) {
   unsigned Opc = Inst.getOpcode();
   const MCInstrDesc &MCID = MII.get(Opc);
 
   if (ForcedVEXEncoding == VEXEncoding_EVEX &&
       (MCID.TSFlags & X86II::EncodingMask) != X86II::EVEX)
     return Match_Unsupported;
 
   if ((ForcedVEXEncoding == VEXEncoding_VEX ||
+       ForcedVEXEncoding == VEXEncoding_VEX2 ||
        ForcedVEXEncoding == VEXEncoding_VEX3) &&
       (MCID.TSFlags & X86II::EncodingMask) != X86II::VEX)
     return Match_Unsupported;
 
   // These instructions match ambiguously with their VEX encoded counterparts
   // and appear first in the matching table. Reject them unless we're forcing
   // EVEX encoding.
   // FIXME: We really need a way to break the ambiguity.
bool X86AsmParser::MatchAndEmitATTInstruction(SMLoc IDLoc, unsigned &Opcode,
   // 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]);
   unsigned Prefixes = getPrefixes(Operands);
 
   MCInst Inst;
 
+  // Passing the prefix info to Printer if VEX or EVEX encoding is forced.
+  if (ForcedVEXEncoding == VEXEncoding_VEX)
+    Prefixes |= X86::Force_VEXEncoding;
+  else if (ForcedVEXEncoding == VEXEncoding_VEX2)
+    Prefixes |= X86::Force_VEX2Encoding;
+  else if (ForcedVEXEncoding == VEXEncoding_EVEX)
+    Prefixes |= X86::Force_EVEXEncoding;
+  else if (ForcedVEXEncoding == VEXEncoding_VEX3) {
     // If VEX3 encoding is forced, we need to pass the USE_VEX3 flag to the
     // encoder.
-  if (ForcedVEXEncoding == VEXEncoding_VEX3)
     Prefixes |= X86::IP_USE_VEX3;
+    Prefixes |= X86::Force_VEX3Encoding;
+  }
 
   // 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/X86BaseInfo.h

enum STATIC_ROUNDING {
     TO_POS_INF = 2,
     TO_ZERO = 3,
     CUR_DIRECTION = 4,
     NO_EXC = 8
   };
 
   /// The constants to describe instr prefixes if there are
   enum IPREFIXES {
     IP_NO_PREFIX = 0,
-    IP_HAS_OP_SIZE = 1,
-    IP_HAS_AD_SIZE = 2,
-    IP_HAS_REPEAT_NE = 4,
-    IP_HAS_REPEAT = 8,
-    IP_HAS_LOCK = 16,
-    IP_HAS_NOTRACK = 32,
-    IP_USE_VEX3 = 64,
-    IP_USE_DISP8 = 128,
-    IP_USE_DISP32 = 256,
+    IP_HAS_OP_SIZE = 1U << 0,
+    IP_HAS_AD_SIZE = 1U << 1,
+    IP_HAS_REPEAT_NE = 1U << 2,
+    IP_HAS_REPEAT = 1U << 3,
+    IP_HAS_LOCK = 1U << 4,
+    IP_HAS_NOTRACK = 1U << 5,
+    IP_USE_VEX3 = 1U << 6,
+    IP_USE_DISP8 = 1U << 7,
+    IP_USE_DISP32 = 1U << 8,
+    Force_VEXEncoding = 1U << 9,
+    Force_VEX2Encoding = 1U << 10,
+    Force_VEX3Encoding = 1U << 11,
+    Force_EVEXEncoding = 1U << 12,
   };
 
   enum OperandType : unsigned {
     /// AVX512 embedded rounding control. This should only have values 0-3.
     OPERAND_ROUNDING_CONTROL = MCOI::OPERAND_FIRST_TARGET,
     OPERAND_COND_CODE,
   };
 

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::Force_VEXEncoding)
+    O << "\t{vex}";
+  else if (Flags & X86::Force_VEX2Encoding)
+    O << "\t{vex2}";
+  else if (Flags & X86::Force_VEX3Encoding)
+    O << "\t{vex3}";
+  else if (Flags & X86::Force_EVEXEncoding)
+    O << "\t{evex}";
 }
 
 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: