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

[Reid Kleckner via llvm-commits]

rnk added inline comments.

================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1057-1058
@@ +1056,4 @@
+    for (int i = 0; i < FinalOperands.size(); ++i) {
+      X86Operand &OrigOp  = (X86Operand &)*OrigOperands[i + 1];
+      X86Operand &FinalOp = (X86Operand &)*FinalOperands[i];
+
----------------
Please use a static_cast here

================
Comment at: test/MC/X86/intel-syntax.s:740
@@ +739,3 @@
+
+ins byte ptr [eax], dx
+// CHECK: insb %dx, %es:(%edi)
----------------
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)

================
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]
----------------
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.


Repository:
  rL LLVM

http://reviews.llvm.org/D14827