X86 MC: Handle instructions like fxsave that match multiple operand sizes
authorReid Kleckner <reid@kleckner.net>
Wed, 27 Aug 2014 20:10:38 +0000 (20:10 +0000)
committerReid Kleckner <reid@kleckner.net>
Wed, 27 Aug 2014 20:10:38 +0000 (20:10 +0000)
Instructions like 'fxsave' and control flow instructions like 'jne'
match any operand size. The loop I added to the Intel syntax matcher
assumed that using a different size would give a different instruction.
Now it handles the case where we get the same instruction for different
memory operand sizes.

This also allows us to remove the hack we had for unsized absolute
memory operands, because we can successfully match things like 'jnz'
without reporting ambiguity.  Removing this hack uncovered test case
involving 'fadd' that was ambiguous. The memory operand could have been
single or double precision.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@216604 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Target/X86/AsmParser/X86AsmParser.cpp
test/MC/X86/intel-syntax-ambiguous.s
test/MC/X86/intel-syntax.s

index fde41aa14cd63b5e28808f82b3f6b387c3c67054..bc8ac26603b7198ccafd98b306b34dba15aa2000 100644 (file)
@@ -2573,10 +2573,7 @@ bool X86AsmParser::MatchAndEmitIntelInstruction(SMLoc IDLoc, unsigned &Opcode,
   X86Operand *UnsizedMemOp = nullptr;
   for (const auto &Op : Operands) {
     X86Operand *X86Op = static_cast<X86Operand *>(Op.get());
-    // FIXME: Remove this exception for absolute memory references. Currently it
-    // allows us to assemble 'call foo', because foo is represented as a memory
-    // operand.
-    if (X86Op->isMemUnsized() && !X86Op->isAbsMem())
+    if (X86Op->isMemUnsized())
       UnsizedMemOp = X86Op;
   }
 
@@ -2602,14 +2599,27 @@ bool X86AsmParser::MatchAndEmitIntelInstruction(SMLoc IDLoc, unsigned &Opcode,
     for (unsigned Size : MopSizes) {
       UnsizedMemOp->Mem.Size = Size;
       uint64_t ErrorInfoIgnore;
-      Match.push_back(MatchInstructionImpl(Operands, Inst, ErrorInfoIgnore,
-                                           MatchingInlineAsm,
-                                           isParsingIntelSyntax()));
+      unsigned LastOpcode = Inst.getOpcode();
+      unsigned M =
+          MatchInstructionImpl(Operands, Inst, ErrorInfoIgnore,
+                               MatchingInlineAsm, isParsingIntelSyntax());
+      if (Match.empty() || LastOpcode != Inst.getOpcode())
+        Match.push_back(M);
+
       // If this returned as a missing feature failure, remember that.
       if (Match.back() == Match_MissingFeature)
         ErrorInfoMissingFeature = ErrorInfoIgnore;
     }
-  } else {
+
+    // Restore the size of the unsized memory operand if we modified it.
+    if (UnsizedMemOp)
+      UnsizedMemOp->Mem.Size = 0;
+  }
+
+  // If we haven't matched anything yet, this is not a basic integer or FPU
+  // operation.  There shouldn't be any ambiguity in our mneumonic table, so try
+  // matching with the unsized operand.
+  if (Match.empty()) {
     Match.push_back(MatchInstructionImpl(Operands, Inst, ErrorInfo,
                                          MatchingInlineAsm,
                                          isParsingIntelSyntax()));
index 1acfb5ba70f96205071b589bfd1edf40dda4e1a7..fe1fe502390297e56667139ae16463cbf4154560 100644 (file)
@@ -42,3 +42,6 @@ add byte ptr [eax], eax
 
 add rax, 3
 // CHECK: error: register %rax is only available in 64-bit mode
+
+fadd   "?half@?0??bar@@YAXXZ@4NA"
+// CHECK: error: ambiguous operand size for instruction 'fadd'
index bb1762e472723ea7b723db0c514baf270bb68d05..325f64d533b1d9e5a9ea0baf3a67326cf6754f56 100644 (file)
@@ -603,8 +603,8 @@ mov rcx, qword ptr [_g0 + 8]
 "?half@?0??bar@@YAXXZ@4NA":
        .quad   4602678819172646912
 
-fadd   "?half@?0??bar@@YAXXZ@4NA"
-fadd   "?half@?0??bar@@YAXXZ@4NA"@IMGREL
+fadd   dword ptr "?half@?0??bar@@YAXXZ@4NA"
+fadd   dword ptr "?half@?0??bar@@YAXXZ@4NA"@IMGREL
 // CHECK: fadds   "?half@?0??bar@@YAXXZ@4NA"
 // CHECK: fadds   "?half@?0??bar@@YAXXZ@4NA"@IMGREL32
 
@@ -641,3 +641,24 @@ fstp dword ptr [rax]
 // CHECK: fstpt (%rax)
 // CHECK: fstpl (%rax)
 // CHECK: fstps (%rax)
+
+fxsave [eax]
+fsave [eax]
+fxrstor [eax]
+frstor [eax]
+// CHECK: fxsave (%eax)
+// CHECK: wait
+// CHECK: fnsave (%eax)
+// CHECK: fxrstor (%eax)
+// CHECK: frstor (%eax)
+
+// FIXME: Should we accept this?  Masm accepts it, but gas does not.
+fxsave dword ptr [eax]
+fsave dword ptr [eax]
+fxrstor dword ptr [eax]
+frstor dword ptr [eax]
+// CHECK: fxsave (%eax)
+// CHECK: wait
+// CHECK: fnsave (%eax)
+// CHECK: fxrstor (%eax)
+// CHECK: frstor (%eax)