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

[Jan Beulich]

On 07.11.2019 18:43,  H.J. Lu  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.

IgnoreSize and Size<N> have different purposes. Combining them
doesn't happen very often, but if you go look you'll find that
there are more cases than just the two insns here. The reason a
similar construct isn't needed for QWORD_MNEM_SUFFIX is simply
because there's no checking of i.tm.opcode_modifier.ignoresize
to begin with. If there was, I'm sure Size would need to be
honored there as well.

>  I much prefer to remove CMPSD and
> MOVSD support with explicit operands from Intel syntax, which aren't
> even in Intel SDM.

Intel SDM isn't the only (or even the main) reference here. MASM
is what we want to be compatible with as much as possible. And
even without that, without explicit operands code needing to set
a segment override or non-default address size reads far more
clumsy.

As an aside - why did you ask to fix the regression (and even
supplied a change to do so on your own) when at the same time
you'd prefer to see these templates go away (which, as said, they
can't).

> We need CMPSD and MOVSD with explicit operands for AT&T syntax only because AT&T
> syntax uses suffixes for memory operand size.

There's no such thing as string insns named CMPSD or MOVSD in AT&T
syntax - it's CMPSL and MOVSL there. CMPSD and MOVSD are solely
SSE2 insns there.

Jan