[PATCH 3/3] Enable Intel MOVDIRI, MOVDIR64B instructions.

[H.J. Lu]

On Thu, Apr 26, 2018 at 11:40 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 26.04.18 at 21:49, <igor.v.tsimbalist@intel.com> wrote:
>>--- /dev/null
>>+++ b/gas/testsuite/gas/i386/x86-64-movdir-intel.d
>>@@ -0,0 +1,18 @@
>>+#as:
>>+#objdump: -dw -Mintel
>>+#name: x86_64 MOVDIR[I,64B] insns (Intel disassembly)
>>+#source: x86-64-movdir.s
>>+
>>+.*: +file format .*
>>+
>>+
>>+Disassembly of section \.text:
>>+
>>+0+ <_start>:
>>+[     ]*[a-f0-9]+:[   ]*48 0f 38 f9 01[       ]*rex\.W movdiri QWORD PTR \[rcx\],rax
>>+[     ]*[a-f0-9]+:[   ]*66 0f 38 f8 01[       ]*movdir64b rax,\[rcx\]
>>+[     ]*[a-f0-9]+:[   ]*67 66 0f 38 f8 01[    ]*movdir64b eax,\[ecx]
>>+[     ]*[a-f0-9]+:[   ]*48 0f 38 f9 01[       ]*rex\.W movdiri QWORD PTR \[rcx\],rax
>
> What are the REX.W doing here (also in the AT&T counterpart)?
>
>>--- a/opcodes/i386-opc.h
>>+++ b/opcodes/i386-opc.h
>>@@ -231,6 +231,10 @@ enum
>>   CpuWAITPKG,
>>   /* CLDEMOTE instruction required */
>>   CpuCLDEMOTE,
>>+  /* MOVDIRI instruction support required */
>>+  CpuMOVDIRI,
>>+  /* MOVDIRR64B instruction required */
>>+  CpuMOVDIR64B,
>>   /* MMX register support required */
>>   CpuRegMMX,
>>   /* XMM register support required */
>
> Considering patch context here, this clearly wasn't re-based onto the
> tip of master before submitting.

Fixed.

>>@@ -628,6 +638,7 @@ static bitfield opcode_modifiers[] =
>>   BITFIELD (ToDword),
>>   BITFIELD (ToQword),
>>   BITFIELD (AddrPrefixOp0),
>>+  BITFIELD (AddrPrefixOpReg),
>
> I'm not convinced this new attribute is needed - I'd much rather see
> AddrPrefixOp0 re-purposed/generalized (and suitably renamed, e.g.
> simply the 0 stripped). This is based on the fact that so far that
> existing attribute is used only in templates not allowing for a memory
> operand at all.

AddrPrefixOp0 has been renamed to AddrPrefixOpReg by

https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=75c0a438994f00240ecd1baca3e3c11cc3b219e5

> H.J., considering the effort I'm putting in to overcome some of the
> easiest-possible-solution things that have been allowed in, I'd much
> appreciate if you would push for at least investigation of whether a
> more elaborate approach exists before allowing in in particular single
> use new insn attributes.
>
>>+movdiri, 2, 0xf38f9, None, 3, CpuMOVDIRI, Modrm|CheckRegSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Reg32, Dword|Qword|Unspecified|BaseIndex|Disp8|Disp32|Disp32S }
>>+movdiri, 2, 0xf38f9, None, 3, CpuMOVDIRI|Cpu64, Modrm|CheckRegSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|Rex64, { Reg64, Dword|Qword|Unspecified|BaseIndex|Disp8|Disp32|Disp32S }
>
> I don't understand why you need two templates here. And if you really
> do, Dword on the 64-bit one looks wrong as do Qword and Disp32S on
> the 32-bit one. Disp16 is missing in any case. I also don't see the point
> of CheckRegSize - there is only a single register operand (the attribute
> has meaning for memory operands only on SIMD templates).

Fixed.

>>+movdir64b, 2, 0x660f38f8, None, 3, CpuMOVDIR64B|CpuNo64, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|AddrPrefixOpReg, { Unspecified|ZMMword|BaseIndex|Disp8|Disp32|Disp32S, Reg16|Reg32 }
>>+movdir64b, 2, 0x660f38f8, None, 3, CpuMOVDIR64B|Cpu64, Modrm|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|NoRex64|AddrPrefixOpReg, { Unspecified|ZMMword|BaseIndex|Disp8|Disp32|Disp32S, Reg32|Reg64 }
>
> Almost all the same here. Additionally, while I can see that ZMMword fits
> the 64-byte operand size, I really think it would look rather odd to have
> "zmmword ptr" used on an operand here. Simply require no operand size
> prefix (in Intel syntax mode), just like you make the disassembler not
> produce any?
>

Fixed.  We need 2 templates since Reg16 isn't supported in 64-bit mode.

This is the patch I am going to check in later.

Thanks.

-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Enable-Intel-MOVDIRI-MOVDIR64B-instruction.patch
Type: text/x-patch
Size: 20890 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20180507/2d1f03b8/attachment.bin>