[llvm-mc] Make the suffix matcher more accurate.

[pengfei]

Summary

Some instruction like VPMULDQ is NOT the variant of VPMULD but a new one
So we should make sure the suffix matcher only works for memory variant
that has the same size with the suffix.
Currently we only check for SSE/AVX* instructions, because many legacy
instructions didn't declare the alias instructions of their variants.

[craig.topper]

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

3461

How about isVectorReg?

[craig.topper]

llvm/lib/Target/X86/AsmParser/X86Operand.h

461

Include MMX?

This is also currently broken

maskmov %mm0, %mm0

should only accept
maskmovq %mm0, %mm0

[pengfei]

Address review comments.

[craig.topper]

I'd like a more robust way to do this, but I don't have any great ideas how. So I guess, LGTM Please put "[X86]" in the commit message before commiting and in review titles for X86 specific things. I have an email filter looking for that. I'd guess others do as well.

[MaskRay]

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

3474

This breaks assembling of movzb 1280(%rbx,%r12), %r12 (!HasVectorReg)

I fixed it in 5b4cd2d4c42360469ccc9f59aa04a1a24b290df9 . There may be further clean-ups.

[pengfei]

Thanks @MaskRay.

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

bool X86AsmParser::MatchAndEmitATTInstruction(SMLoc IDLoc, unsigned &Opcode,
 
   // If this instruction starts with an 'f', then it is a floating point stack
   // instruction.  These come in up to three forms for 32-bit, 64-bit, and
   // 80-bit floating point, which use the suffixes s,l,t respectively.
   //
   // Otherwise, we assume that this may be an integer instruction, which comes
   // in 8/16/32/64-bit forms using the b,w,l,q suffixes respectively.
   const char *Suffixes = Base[0] != 'f' ? "bwlq" : "slt\0";
+  // MemSize corresponding to Suffixes.  { 8, 16, 32, 64 }    { 32, 64, 80, 0 }
+  const char *MemSize = Base[0] != 'f' ? "\x08\x10\x20\x40" : "\x20\x40\x50\0";
 
   // Check for the various suffix matches.
   uint64_t ErrorInfoIgnore;
   FeatureBitset ErrorInfoMissingFeatures; // Init suppresses compiler warnings.
   unsigned Match[4];
 
+  // Some instruction like VPMULDQ is NOT the variant of VPMULD but a new one.
+  // So we should make sure the suffix matcher only works for memory variant
+  // that has the same size with the suffix.
+  // FIXME: This flag is a workaround for legacy instructions that didn't
+  // declare non suffix variant assembly.
+  bool HasVectorReg = false;
+  X86Operand *MemOp = nullptr;
+  for (const auto &Op : Operands) {
+    X86Operand *X86Op = static_cast<X86Operand *>(Op.get());
+    if (X86Op->isVectorReg())
+      HasVectorReg = true;
+    else if (X86Op->isMem()) {
+      MemOp = X86Op;
+      assert(MemOp->Mem.Size == 0 && "Memory size always 0 under ATT syntax");
+      // Have we found an unqualified memory operand,
+      // break. IA allows only one memory operand.
+      break;
+    }
+  }
+
   for (unsigned I = 0, E = array_lengthof(Match); I != E; ++I) {
     Tmp.back() = Suffixes[I];
-    Match[I] = MatchInstruction(Operands, Inst, ErrorInfoIgnore,
-                                MissingFeatures, MatchingInlineAsm,
-                                isParsingIntelSyntax());
+    if (MemOp)
+      MemOp->Mem.Size = MemSize[I];
+    Match[I] = Match_MnemonicFail;
+    if (MemOp || !HasVectorReg) {
+      Match[I] =
+          MatchInstruction(Operands, Inst, ErrorInfoIgnore, MissingFeatures,
+                           MatchingInlineAsm, isParsingIntelSyntax());
       // If this returned as a missing feature failure, remember that.
       if (Match[I] == Match_MissingFeature)
         ErrorInfoMissingFeatures = MissingFeatures;
     }
+  }
 
   // Restore the old token.
   Op.setTokenValue(Base);
 
   // If exactly one matched, then we treat that as a successful match (and the
   // instruction will already have been filled in correctly, since the failing
   // matches won't have modified it).
   unsigned NumSuccessfulMatches =

llvm/lib/Target/X86/AsmParser/X86Operand.h

struct X86Operand final : public MCParsedAsmOperand {
   bool isDXReg() const { return Kind == DXRegister; }
 
   bool isGR32orGR64() const {
     return Kind == Register &&
       (X86MCRegisterClasses[X86::GR32RegClassID].contains(getReg()) ||
       X86MCRegisterClasses[X86::GR64RegClassID].contains(getReg()));
   }
 
+  bool isVectorReg() const {
+    return Kind == Register &&
+           (X86MCRegisterClasses[X86::VR64RegClassID].contains(getReg()) ||
+            X86MCRegisterClasses[X86::VR128XRegClassID].contains(getReg()) ||
+            X86MCRegisterClasses[X86::VR256XRegClassID].contains(getReg()) ||
+            X86MCRegisterClasses[X86::VR512RegClassID].contains(getReg()));
+  }
+
   bool isVK1Pair() const {
     return Kind == Register &&
       X86MCRegisterClasses[X86::VK1RegClassID].contains(getReg());
   }
 
   bool isVK2Pair() const {
     return Kind == Register &&
       X86MCRegisterClasses[X86::VK2RegClassID].contains(getReg());

llvm/test/MC/X86/avx512-err.s

 // RUN: not llvm-mc %s -triple x86_64-unknown-unknown --show-encoding -o /dev/null 2>&1 | FileCheck --check-prefix=ERR %s
 
 // ERR: Register k0 can't be used as write mask
 vpcmpd $1, %zmm24, %zmm7, %k5{%k0}
 
 // ERR: Expected a {z} mark at this point
 vfmsub213ps %zmm8, %zmm8, %zmm8{%k2} {rn-sae}
 
 // ERR: Expected an op-mask register at this point
 vfmsub213ps %zmm8, %zmm8, %zmm8 {rn-sae}
 
 // ERR: invalid operand for instruction
 cvtsd2sil  {rn-sae}, %xmm1, %eax
 
 // ERR: Expected an identifier after {
 cvtsd2sil  {{sae}, %xmm1, %eax
+
+// ERR: invalid instruction mnemonic 'vpmuld'
+vpmuld %xmm1, %xmm2, %xmm3
+
+// ERR: invalid instruction mnemonic 'maskmov'
+maskmov %mm1, %mm2

llvm/test/tools/llvm-mca/X86/BdVer2/dependent-pmuld-paddd.s

 # NOTE: Assertions have been autogenerated by utils/update_mca_test_checks.py
 # RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=bdver2 -iterations=500 -timeline < %s | FileCheck %s
 
-vpmuld %xmm0, %xmm0, %xmm1
+vpmuldq %xmm0, %xmm0, %xmm1
 vpaddd %xmm1, %xmm1, %xmm0
 vpaddd %xmm0, %xmm0, %xmm3
 
 # CHECK:      Iterations:        500
 # CHECK-NEXT: Instructions:      1500
 # CHECK-NEXT: Total Cycles:      3005
 # CHECK-NEXT: Total uOps:        1500
 

llvm/test/tools/llvm-mca/X86/BtVer2/dependent-pmuld-paddd.s

 # NOTE: Assertions have been autogenerated by utils/update_mca_test_checks.py
 # RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -iterations=500 -timeline < %s | FileCheck %s
 
-vpmuld %xmm0, %xmm0, %xmm1
+vpmuldq %xmm0, %xmm0, %xmm1
 vpaddd %xmm1, %xmm1, %xmm0
 vpaddd %xmm0, %xmm0, %xmm3
 
 # CHECK:      Iterations:        500
 # CHECK-NEXT: Instructions:      1500
 # CHECK-NEXT: Total Cycles:      1504
 # CHECK-NEXT: Total uOps:        1500