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

[H.J. Lu]

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.

> 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
>


-- 
H.J.