Home Original page

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

Jan Beulich jbeulich@suse.com
Mon Feb 3 08:38:00 GMT 2020 
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 is because you added No_lSuf to the existing template.
Obviously the No_wSuf / No_lSuf present on the two new templates
then are similarly wrong. Just like for MOVSX/MOVZX in AT&T mode
a suffix should be permitted imo (even if there's no ambiguity
here), specifying the size of the source (memory) operand. An
alternative would be to make MOVSX/MOVZX strictly Intel syntax
mode only, but I guess there would then be a fair risk of
breaking existing code.

I need to know which direction to go in order to sensibly rebase
that patch as well as the other remaining ones from that series.

Jan
More information about the Binutils mailing list