[PATCH] D16359: [X86] - Removing warning on legal cases caused by commit r258132

[Marina Yatsina via llvm-commits]

myatsina created this revision.
myatsina added reviewers: rnk, sberg, craig.topper.
myatsina added a subscriber: llvm-commits.
myatsina set the repository for this revision to rL LLVM.

There's an overloading of the "movsd" and "cmpsd" instructions, e.g. movsd can be either "Move Data from String to String" or "Move or Merge Scalar Double-Precision Floating-Point Value".
The former should produce warnings when parsing a memory operand that is not ESI/EDI, but the latter should not.

Fixed the code to produce warnings only after making sure we're dealing with the first case.

Expanded the tests of the produced warnings + fixed RUN line the test so that it would check the both stdout and stderr


Repository:
  rL LLVM

http://reviews.llvm.org/D16359

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

Index: test/MC/X86/intel-syntax.s
===================================================================
--- test/MC/X86/intel-syntax.s
+++ test/MC/X86/intel-syntax.s
@@ -1,4 +1,6 @@
-// RUN: llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel %s | FileCheck %s
+// RUN: llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel %s > %t 2> %t.err
+// RUN: FileCheck < %t %s
+// RUN: FileCheck --check-prefix=CHECK-STDERR < %t.err %s
 
 _test:
 	xor	EAX, EAX
@@ -755,26 +757,39 @@
 ins byte ptr [eax], dx
 // CHECK: insb %dx, %es:(%edi)
 // CHECK-STDERR: memory operand is only for determining the size, ES:(R|E)DI will be used for the location
+// CHECK-STDERR-NEXT: ins byte ptr [eax], dx
 outs dx, word ptr [eax]
 // CHECK: outsw (%esi), %dx
 // CHECK-STDERR: memory operand is only for determining the size, ES:(R|E)SI will be used for the location
+// CHECK-STDERR-NEXT: outs dx, word ptr [eax]
 lods dword ptr [eax]
 // CHECK: lodsl (%esi), %eax
 // CHECK-STDERR: memory operand is only for determining the size, ES:(R|E)SI will be used for the location
+// CHECK-STDERR-NEXT: lods dword ptr [eax]
 stos qword ptr [eax]
 // CHECK: stosq %rax, %es:(%edi)
 // CHECK-STDERR: memory operand is only for determining the size, ES:(R|E)DI will be used for the location
+// CHECK-STDERR-NEXT: stos qword ptr [eax]
 scas byte ptr [eax]
 // CHECK: scasb %es:(%edi), %al
 // CHECK-STDERR: memory operand is only for determining the size, ES:(R|E)DI will be used for the location
+// CHECK-STDERR-NEXT: scas byte ptr [eax]
 cmps word ptr [eax], word ptr [ebx]
 // CHECK: cmpsw %es:(%edi), (%esi)
 // CHECK-STDERR: memory operand is only for determining the size, ES:(R|E)SI will be used for the location
+// CHECK-STDERR-NEXT: cmps word ptr [eax], word ptr [ebx]
 // CHECK-STDERR: memory operand is only for determining the size, ES:(R|E)DI will be used for the location
+// CHECK-STDERR-NEXT: cmps word ptr [eax], word ptr [ebx]
 movs dword ptr [eax], dword ptr [ebx]
 // CHECK: movsl (%esi), %es:(%edi)
 // CHECK-STDERR: memory operand is only for determining the size, ES:(R|E)DI will be used for the location
+// CHECK-STDERR-NEXT: movs dword ptr [eax], dword ptr [ebx]
 // CHECK-STDERR: memory operand is only for determining the size, ES:(R|E)SI will be used for the location
+// CHECK-STDERR-NEXT: movs dword ptr [eax], dword ptr [ebx]
+
+movsd  qword ptr [rax], xmm0
+// CHECK: movsd %xmm0, (%rax)
+// CHECK-STDERR-NOT: movsd qword ptr [rax], xmm0
 
 xlat byte ptr [eax]
 // CHECK: xlatb
Index: lib/Target/X86/AsmParser/X86AsmParser.cpp
===================================================================
--- lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -1056,6 +1056,7 @@
     assert(OrigOperands.size() == FinalOperands.size() + 1 &&
            "Opernand size mismatch");
 
+    SmallVector<std::pair<SMLoc, std::string>, 2> Warnings;
     // Verify types match
     int RegClassID = -1;
     for (unsigned int i = 0; i < FinalOperands.size(); ++i) {
@@ -1100,18 +1101,26 @@
 
         if (FinalReg != OrigReg) {
           std::string RegName = IsSI ? "ES:(R|E)SI" : "ES:(R|E)DI";
-          Warning(OrigOp.getStartLoc(),
-                  "memory operand is only for determining the size, " +
-                      RegName + " will be used for the location");
+          Warnings.push_back(std::make_pair(
+              OrigOp.getStartLoc(),
+              "memory operand is only for determining the size, " + RegName +
+                  " will be used for the location"));
         }
 
         FinalOp.Mem.Size = OrigOp.Mem.Size;
         FinalOp.Mem.SegReg = OrigOp.Mem.SegReg;
         FinalOp.Mem.BaseReg = FinalReg;
       }
     }
 
-    // Remove old operandss
+    // Produce warnings only if all the operands passed the adjustment - prevent
+    // legal cases like "movsd (%rax), %xmm0" mistakenly produce warnings
+    for (auto WarningMsg = Warnings.begin(); WarningMsg < Warnings.end();
+         ++WarningMsg) {
+      Warning((*WarningMsg).first, (*WarningMsg).second);
+    }
+
+    // Remove old operands
     for (unsigned int i = 0; i < FinalOperands.size(); ++i)
       OrigOperands.pop_back();
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D16359.45396.patch
Type: text/x-patch
Size: 4187 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160120/b4f251a4/attachment.bin>