[PATCH] x86-64: Properly encode and decode movsxd

[Jan Beulich]

On 03.02.2020 15:49, H.J. Lu wrote:
> On Mon, Feb 3, 2020 at 12:38 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 26.01.2020 16:00, H.J. Lu wrote:
>>> On Fri, Jan 24, 2020 at 3:51 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>
>>>> On Fri, Jan 24, 2020 at 12:21 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>
>>>>> On 23.01.2020 21:24, H.J. Lu wrote:
>>>>>> movsxd is a 64-bit only instruction.  It supports both 16-bit and 32-bit
>>>>>> destination registers.  Its AT&T mnemonic is movslq which only supports
>>>>>> 64-bit destination register.  There is also a discrepancy between AMD64
>>>>>> and Intel64 on movsxd with 16-bit destination register.  AMD64 supports
>>>>>> 32-bit source operand and Intel64 supports 16-bit source operand.
>>>>>>
>>>>>> This patch updates movsxd encoding and decoding to alow 16-bit and 32-bit
>>>>>> destination registers.  It also handles movsxd with 16-bit destination
>>>>>> register for AMD64 and Intel 64.
>>>>>
>>>>> I appreciate you taking care of the disassembler and documentation
>>>>> side, but would you mind pointing out what's wrong with the previously
>>>>> posted patches making the assembler deal with this correctly? For
>>>>> example, I get away ...
>>>>>
>>>>>> --- a/gas/config/tc-i386.c
>>>>>> +++ b/gas/config/tc-i386.c
>>>>>> @@ -6690,7 +6690,9 @@ check_long_reg (void)
>>>>>>            && i.tm.operand_types[op].bitfield.dword)
>>>>>>        {
>>>>>>       if (intel_syntax
>>>>>> -         && i.tm.opcode_modifier.toqword
>>>>>> +         && (i.tm.opcode_modifier.toqword
>>>>>> +             /* Also convert to QWORD for MOVSXD.  */
>>>>>> +             || i.tm.base_opcode == 0x63)
>>>>>>           && i.types[0].bitfield.class != RegSIMD)
>>>>>
>>>>> ... without such a (pretty arbitrary) adjustment (as an aside: the
>>>>> comment is slightly misleading in that the change also affects some
>>>>> MOVSX templates), while ...
>>>>
>>>> My patch in assembler
>>>>
>>>> 1.  Keep movslq unchanged, which only allows 64-bit destination register.
>>>> 2.  Allow DWORD source with movsxd.
>>>>
>>>> My testcases cover the above.  Without the assembler change, I got
>>>>
>>>> FAIL: x86-64 movsxd (AMD64)
>>>> FAIL: x86-64 movsxd (AMD64) (Intel mode)
>>>> FAIL: x86-64 movsxd (Intel64)
>>>> FAIL: x86-64 movsxd (Intel64) (Intel mode)
>>>>
>>>
>>> I am checking in this patch tomorrow.
>>
>> And you broke previously working code, which has no testcase so
>> far, but which
>> https://sourceware.org/ml/binutils/2019-12/msg00354.html
>> adds a test for:
>>
>> movsxdl (%rax),%rcx
> 
> This isn't valid AT&T mnemonic.  No one should use it.

Mind telling me what you derive this from? It is my understanding
that the general rule of how to derive AT&T mnemonics is to take
the Intel manual's mnemonic and add a set of suffixes if varying
operand sizes are permitted. I can accept _new_ insns (in
particular SIMD ones) to not get suffixes supported when they're
not really needed. But this is an original x86-64 GPR insn. It
should be consistent with other original x86-64 GPR insns.

Furthermore, if it really was intentional for your commit to
remove support for the suffix, then why does the description not
say so (nor, what's perhaps even worse, the ChangeLog entry)?

Jan