Re: [PATCH 1/2] x86/Intel: make sure MOVSD/CMPSD have their Size32 honored

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Thu, Nov 7, 2019 at 9:43 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Thu, Nov 7, 2019 at 2:18 AM Jan Beulich <jbeulich@suse.com> wrote:
> >
> > The combination of IgnoreSize and Size32 (or Size16) didn't work right:
> > While at the top of process_suffix() Size<N> carefully get honored to
> > force a respective suffix, code later in the function ignored the
> > attribute when determining whether to emit an insn size prefix. This
> > fixes a regression from d241b91073 ("x86/Intel: correct MOVSD and CMPSD
> > handling"). (The Size16 aspect of this can be observed when, just for
> > this purpose, also adding explicit "movsw" templates. The lack of an
> > IgnoreSize check in 'q' suffix handling means that no such issue
> > existed even if explicit "movsq" templates were added.)
> >
> > Also further extend the test cases added/extended by that commit.
> >
> > gas/
> > 2019-11-07  Jan Beulich  <jbeulich@suse.com>
> >
> >         PR/gas 25167
> >         * config/tc-i386.c (process_suffix): Honor size attribute when
> >         checking whether to add operand size prefix.
> >         * testsuite/gas/i386/intel-cmps.s,
> >         testsuite/gas/i386/intel-movs.s: Extend.
> >         * testsuite/gas/i386/intel-cmps32.d,
> >         testsuite/gas/i386/intel-cmps64.d,
> >         testsuite/gas/i386/intel-movs32.d,
> >         testsuite/gas/i386/intel-movs64.d: Adjust expectations.
> >         * testsuite/gas/i386/intel-cmps16.d,
> >         testsuite/gas/i386/intel-movs16.d: New.
> >         * testsuite/gas/i386/i386.exp: Run new tests.
> >
> > --- a/gas/config/tc-i386.c
> > +++ b/gas/config/tc-i386.c
> > @@ -6466,7 +6466,8 @@ process_suffix (void)
> >               return 0;
> >         }
> >        else if (i.suffix != QWORD_MNEM_SUFFIX
> > -              && !i.tm.opcode_modifier.ignoresize
> > +              && (!i.tm.opcode_modifier.ignoresize
> > +                  || i.tm.opcode_modifier.size)
> >                && !i.tm.opcode_modifier.floatmf
> >                && !is_any_vex_encoding (&i.tm)
> >                && ((i.suffix == LONG_MNEM_SUFFIX) == (flag_code == CODE_16BIT)
>
> Ignore IgnoreSize with Size looks very odd.  I much prefer to remove CMPSD and
> MOVSD support with explicit operands from Intel syntax, which aren't
> even in Intel SDM.
> We need CMPSD and MOVSD with explicit operands for AT&T syntax only because AT&T
> syntax uses suffixes for memory operand size.
>

Another problem is Intel syntax sets instruction suffix from operand size:

[hjl@gnu-cfl-1 gas]$ cat 2.s
.intel_syntax noprefix
call WORD PTR [rax]
[hjl@gnu-cfl-1 gas]$ as -mintel64 -o 2.o 2.s
2.s: Assembler messages:
2.s:2: Error: invalid instruction suffix for `call'
[hjl@gnu-cfl-1 gas]$

It is operand size mismatch, not invalid instruction suffix.  In case
of CMPSD and
and MOVSD,the 'l' suffix is invalid, not DWORD on memory operand.  But suffix
check treats them the same.  This makes Intel syntax more complicated.   Should
Intel syntax disallow suffix in mnemonic?  It will make Intel syntax
closer to SDM.


-- 
H.J.