[PATCH v7] x86: replace adhoc (partly wrong) ambiguous operand checking for MOVSX/MOVZX

[Jan Beulich]

On 13.02.2020 13:19, H.J. Lu wrote:
> 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)

I certainly didn't get any failures. Mind being more specific? Is this
perhaps for an unusual target? Knowing what exactly at least some of the
failures are would be helpful in case I can't repro it myself.

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

Why "disallow movsxb, but allow movsxw"? What do you refer to?
The list above all assembles entirely fine for me. Together with
the test failures I wonder whether you have a broken build.

> I don't think Intel syntax should allow suffix in this case.

This is a consistency thing again - if you don't want to allow
suffixes here, they also shouldn't be allowed on other insns
where they aren't really needed (add, mov, etc). This isn't the
subject of the patch here.

Jan