This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH v7] x86: replace adhoc (partly wrong) ambiguous operand checking for MOVSX/MOVZX
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Jan Beulich <jbeulich at suse dot com>
- Cc: "binutils at sourceware dot org" <binutils at sourceware dot org>
- Date: Thu, 13 Feb 2020 04:19:21 -0800
- Subject: Re: [PATCH v7] x86: replace adhoc (partly wrong) ambiguous operand checking for MOVSX/MOVZX
- References: <21a10238-945a-2a60-9c31-11ce052ff34a@suse.com>
On Thu, Feb 13, 2020 at 1:21 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> For these to get treatment consistent with other operand size checking
> the special logic shouldn't live in md_assemble(), but process_suffix().
> And there's more logic involved than simply zapping the suffix, in
> particular to enforce the general "suffix trumps register size" rule in
> AT&T mode. The cases where behavior is being corrected can be seen from
> the testcase adjustments (as mentioned in the commit introducing these
> tests some cases had wrong expectations at that point, but it seemed
> better to separate testcase introduction from actual code changes).
>
> Note however that MOVS[BW]* and MOVZ[BW]* still won't be fully
> consistent, due to the objection to fold MOVS* templates just like was
> done for MOVZ* in c07315e0c6 ("x86: allow suffix-less movzw and 64-bit
> movzb").
>
> Note further that the assembler change points out (and this patch fixes)
> a wrong Intel syntax test introduced by bc31405ebb2c ("x86-64: Properly
> encode and decode movsxd"): When source code specifies a 16-bit
> destination register, disassembly expectations shouldn't have been to
> find a 32-bit one.
>
> gas/
> 2020-02-XX Jan Beulich <jbeulich@suse.com>
>
> PR gas/25438
> * config/tc-i386.c (md_assemble): Move movsx/movzx special
> casing ...
> (process_suffix): ... here. Consider just the first operand
> initially.
> (check_long_reg): Drop opcode 0x63 special case again.
> * testsuite/gas/i386/i386.s, testsuite/gas/i386/iamcu-1.s,
> testsuite/gas/i386/ilp32/x86-64.s, testsuite/gas/i386/x86_64.s:
> Move ambiguous operand size tests ...
> * testsuite/gas/i386/noreg16.s, testsuite/gas/i386/noreg32.s,
> testsuite/gas/i386/noreg64.s: ... here.
> * testsuite/gas/i386/i386.d, testsuite/gas/i386/i386-intel.d
> testsuite/gas/i386/iamcu-1.d, testsuite/gas/i386/ilp32/x86-64.d,
> testsuite/gas/i386/k1om.d, testsuite/gas/i386/l1om.d,
> testsuite/gas/i386/movx16.l, testsuite/gas/i386/movx32.l,
> testsuite/gas/i386/movx64.l, testsuite/gas/i386/noreg16.d,
> testsuite/gas/i386/noreg16.l, testsuite/gas/i386/noreg32.d,
> testsuite/gas/i386/noreg32.l, testsuite/gas/i386/noreg64.d,
> testsuite/gas/i386/noreg64.l, testsuite/gas/i386/x86-64-movsxd.d,
> testsuite/gas/i386/x86-64-movsxd-intel.d,
> testsuite/gas/i386/x86_64.d, testsuite/gas/i386/x86_64-intel.d:
> Adjust expectations.
> * testsuite/gas/i386/movx16.s, testsuite/gas/i386/movx16.l,
> testsuite/gas/i386/movx32.s, testsuite/gas/i386/movx32.l,
> testsuite/gas/i386/movx64.s, testsuite/gas/i386/movx64.l: New.
> * testsuite/gas/i386/i386.exp: Run new tests.
>
> opcodes/
> 2020-02-XX Jan Beulich <jbeulich@suse.com>
>
> PR gas/25438
> * i386-opc.tbl (movsx): Fold patterns. Also allow Reg32 as
> destination for Cpu64-only variant.
> (movsxd): Also allow Reg32 as destination. Drop Rex64.
> (movzx): Fold patterns.
> * i386-tbl.h: Re-generate.
I got
FAIL: i386 intel
FAIL: i386 intel
FAIL: i386 intel16
FAIL: i386 intel-ok
FAIL: i386 noreg16
FAIL: i386 noreg32
FAIL: i386 movx16
FAIL: i386 movx32
FAIL: i386
FAIL: i386 (Intel mode)
FAIL: gas/i386/iamcu-1
FAIL: x86_64
FAIL: x86-64 (Intel mode)
FAIL: i386 noreg64
FAIL: i386 movx64
FAIL: l1om
FAIL: k1om
FAIL: x86-64 (ILP32)
Also should
movsxb ax, BYTE PTR [rax]
movsxb eax, BYTE PTR [rax]
movsxb rax, BYTE PTR [rax]
movsxw eax, WORD PTR [rax]
movsxw rax, WORD PTR [rax]
movzxb ax, BYTE PTR [rax]
movzxb eax, BYTE PTR [rax]
movzxb rax, BYTE PTR [rax]
movzxw eax, WORD PTR [rax]
movzxw rax, WORD PTR [rax]
be alllowed? It is odd to disallow movsxb, but allow movsxw. I don't
think Intel syntax
should allow suffix in this case.
--
H.J.