[PATCH] D14827: Adding support for missing variations of X86 string related instructions

[Marina Yatsina via llvm-commits]

myatsina added inline comments.

================
Comment at: test/MC/X86/intel-syntax.s:740-741
@@ +739,4 @@
+
+ins byte ptr [eax], dx
+// CHECK: insb %dx, %es:(%edi)
+outs dx, word ptr [eax]
----------------
rnk wrote:
> rnk wrote:
> > myatsina wrote:
> > > rnk wrote:
> > > > I'm confused. Why are these two assembly instructions equivalent? Ditto for the rest, they don't look equivalent.
> > > If you look at the X86 spec of these instructions:
> > > 
> > > INS m8, DX - Input byte from I/O port specified in DX into memory location specified in ES:(E)DI or RDI
> > > which is actually the same as the definition of the "insb" instruction.
> > > The mem operand has no meaning other than distinguishing if it's insb, insw or insl (in this case it's mem8, so it should be insb).
> > > 
> > > "insb" and "ins m8, DX" are equivalent and should map to the same instruction.
> > > 
> > > Does this answer your question?
> > > 
> > > 
> > OK, so the register eax is essentially discarded and is only used to size the implicit memory operand. binutils 'as' issues a warning in this case, but MSVC does not. What does Intel's assembler do? I would like it if we issued a warning that EDI or ESI was expected in place of the user's register.
> You should test what happens with different pointer sizes. Since this is a 64-bit test, can you add a test like this:
>   ins byte ptr [rax], dx
> GAS assembles that to this:
>   insb   (%dx),%es:(%rdi)
> 
> And then again in a separate 32-bit test file, you should do something like:
>   insb byte ptr [ax], dx
>   insb byte ptr [eax], dx
>   // CHECK: insb   (%dx),%es:(%di)
>   // CHECK: insb   (%dx),%es:(%edi)
Intel assembler does not issue a warning, same as MSVC.
I think we shouldn't issue a warning either because according to x86 spec this is a completely legal instruction which has the semantics of looking at ESI/EDI.
The spec does not have any restrictions about the mem operand and does not "expect" ESI/EDI.

================
Comment at: test/MC/X86/intel-syntax.s:740
@@ +739,3 @@
+
+ins byte ptr [eax], dx
+// CHECK: insb %dx, %es:(%edi)
----------------
Added test cases in index-operations.s
I've added only for insw, I believe these cases with the case here have enough coverage


Repository:
  rL LLVM

http://reviews.llvm.org/D14827