[llvm] r258393 - [X86] - Removing warning on legal cases caused by commit r258132

Marina Yatsina via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 21 03:37:07 PST 2016


Author: myatsina
Date: Thu Jan 21 05:37:06 2016
New Revision: 258393

URL: http://llvm.org/viewvc/llvm-project?rev=258393&view=rev
Log:
[X86] - Removing warning on legal cases caused by commit r258132

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 of the test so that it would check both stdout and stderr

Differential Revision: http://reviews.llvm.org/D16359


Modified:
    llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp
    llvm/trunk/test/MC/X86/intel-syntax.s

Modified: llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp?rev=258393&r1=258392&r2=258393&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp (original)
+++ llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp Thu Jan 21 05:37:06 2016
@@ -1056,6 +1056,7 @@ bool X86AsmParser::VerifyAndAdjustOperan
     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,9 +1101,10 @@ bool X86AsmParser::VerifyAndAdjustOperan
 
         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;
@@ -1111,7 +1113,14 @@ bool X86AsmParser::VerifyAndAdjustOperan
       }
     }
 
-    // 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();
   }

Modified: llvm/trunk/test/MC/X86/intel-syntax.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/intel-syntax.s?rev=258393&r1=258392&r2=258393&view=diff
==============================================================================
--- llvm/trunk/test/MC/X86/intel-syntax.s (original)
+++ llvm/trunk/test/MC/X86/intel-syntax.s Thu Jan 21 05:37:06 2016
@@ -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 @@ sidt fword ptr [eax]
 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




More information about the llvm-commits mailing list