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

[H.J. Lu]

On Wed, Feb 12, 2020 at 5:19 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 12.02.2020 13:50, H.J. Lu wrote:
> > On Wed, Feb 12, 2020 at 2:06 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/movx16.l, testsuite/gas/i386/movx32.l,
> >>         testsuite/gas/i386/movx64.l, testsuite/gas/i386/noreg16.l,
> >>         testsuite/gas/i386/noreg32.l, testsuite/gas/i386/noreg64.l,
> >>         testsuite/gas/i386/x86-64-movsxd-intel.d,
> >>         testsuite/gas/i386/x86-64-movsxd.d: Adjust expectations.
> >>
> >> 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.
> >
> > Please combine patch 4 and patch 5 into a single patch.
>
> As indicated before, I'll be fine doing so for committing, but I
> don't think it should be combined any earlier (for making review
> harder). Please clarify whether your response is meant to indicate
> "Okay when folded".

It is harder to review when tests are moved and changed later.
Please squash the second patch into the first one just for review
purpose.

> >   But  we need
> > to first change incorrect register size from warning to error.
>
> We did already, didn't we, by adjusting check_{byte,word,long}_reg()?
>

That is good to know.

-- 
H.J.