[PATCH v3 01/10] x86: improve handling of insns with ambiguous operand sizes

[Jan Beulich]

On 10.01.2020 21:45, H.J. Lu wrote:
> On Fri, Jan 10, 2020 at 12:54 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 09.01.2020 23:35,  H.J. Lu  wrote:
>>> On Fri, Dec 27, 2019 at 1:22 AM Jan Beulich <JBeulich@suse.com> wrote:
>>>>
>>>> Commit b76bc5d54e ("x86: don't default variable shift count insns to
>>>> 8-bit operand size") pointed out a very bad case, but the underlying
>>>> problem is, as mentioned on various occasions, much larger: Silently
>>>> selecting a (nowhere documented afaict) certain default operand size
>>>> when there's no "sizing" suffix and no suitable register operand(s) is
>>>> simply dangerous (for the programmer to make mistakes).
>>>>
>>>> While in Intel syntax mode such mistakes already lead to an error (which
>>>> is going to remain that way), AT&T syntax mode now gains warnings in
>>>> such cases by default, which can be suppressed or promoted to an error
>>>> if so desired by the programmer. Furthermore at least general purpose
>>>> insns now consistently have a default applied (alongside the warning
>>>> emission), rather than accepting some and refusing others.
>>>>
>>>> No warnings are (as before) to be generated for "DefaultSize" insns as
>>>> well as ones acting on selector and other fixed-width values. For
>>>> SYSRET, however, the DefaultSize needs to be dropped - it had been
>>>> wrongly put there in the first place, as it's unrelated to .code16gcc
>>>> (no stack accesses involved).
>>>>
>>>> As set forth as a prereq when I first mentioned this intended change a
>>>> few years back, Linux as well as gcc have meanwhile been patched to
>>>> avoid (emission of) ambiguous operands (and hence triggering of the new
>>>> warning).
>>>>
>>>> Note that I think that in 64-bit mode IRET and far RET would better get
>>>> a diagnostic too, as it's reasonably likely that a suffix-less instance
>>>> really is meant to be a 64-bit one. But I guess I better make this a
>>>> separate follow-on patch.
>>>>
>>>> Note further that floating point operations with integer operands are an
>>>> exception for now: They continue to use short (16-bit) operands by
>>>> default even in 32- and 64-bit modes.
>>>>
>>>> gas/
>>>> 2020-01-XX  Jan Beulich  <jbeulich@suse.com>
>>>>
>>>>         * config/tc-i386.c (process_suffix): Drop SYSRET special case
>>>>         and an intel_syntax check. Re-write lack-of-suffix processing
>>>>         logic.
>>>>         * doc/c-i386.texi: Document operand size defaults for suffix-
>>>>         less AT&T syntax insns.
>>>>         * testsuite/gas/i386/bundle.s, testsuite/gas/i386/lock-1.s,
>>>>         testsuite/gas/i386/opcode.s, testsuite/gas/i386/sse3.s,
>>>>         testsuite/gas/i386/x86-64-avx-scalar.s,
>>>>         testsuite/gas/i386/x86-64-avx.s,
>>>>         testsuite/gas/i386/x86-64-bundle.s,
>>>>         testsuite/gas/i386/x86-64-intel64.s,
>>>>         testsuite/gas/i386/x86-64-lock-1.s,
>>>>         testsuite/gas/i386/x86-64-opcode.s,
>>>>         testsuite/gas/i386/x86-64-sse2avx.s,
>>>>         testsuite/gas/i386/x86-64-sse3.s: Add missing suffixes.
>>>>         * testsuite/gas/i386/nops.s, testsuite/gas/i386/sse-noavx.s,
>>>>         testsuite/gas/i386/x86-64-nops.s,
>>>>         testsuite/gas/i386/x86-64-ptwrite.s,
>>>>         testsuite/gas/i386/x86-64-simd.s,
>>>>         testsuite/gas/i386/x86-64-sse-noavx.s,
>>>>         testsuite/gas/i386/x86-64-suffix.s: Drop bogus suffix-less
>>>>         insns.
>>>>         * testsuite/gas/i386/noreg16.s, testsuite/gas/i386/noreg32.s,
>>>>         testsuite/gas/i386/noreg64.s: Add further tests.
>>>>         * testsuite/gas/i386/ilp32/x86-64-nops.d,
>>>>         testsuite/gas/i386/nops.d, testsuite/gas/i386/noreg16.d,
>>>>         testsuite/gas/i386/noreg32.d, testsuite/gas/i386/noreg64.d,
>>>>         testsuite/gas/i386/sse-noavx.d,
>>>>         testsuite/gas/i386/x86-64-intel64.d,
>>>>         testsuite/gas/i386/x86-64-nops.d,
>>>>         testsuite/gas/i386/x86-64-opcode.d,
>>>>         testsuite/gas/i386/x86-64-ptwrite-intel.d,
>>>>         testsuite/gas/i386/x86-64-ptwrite.d,
>>>>         testsuite/gas/i386/x86-64-simd-intel.d,
>>>>         testsuite/gas/i386/x86-64-simd-suffix.d,
>>>>         testsuite/gas/i386/x86-64-simd.d,
>>>>         testsuite/gas/i386/x86-64-sse-noavx.d
>>>>         testsuite/gas/i386/x86-64-suffix.d,
>>>>         testsuite/gas/i386/x86-64-suffix-intel.d: Adjust expectations.
>>>>         * testsuite/gas/i386/noreg16.l, testsuite/gas/i386/noreg32.l,
>>>>         testsuite/gas/i386/noreg64.l: New.
>>>>         * testsuite/gas/i386/i386.exp: Run new tests.
>>>>
>>>> opcodes/
>>>> 2020-01-XX  Jan Beulich  <jbeulich@suse.com>
>>>>
>>>>         * i386-opc.tbl (sysret): Drop DefaultSize.
>>>>         * i386-tbl.h: Re-generate.
>>>> ---
>>>> v3: Don't add any new DefaultSize. Also diagnose SYSRET in 64-bit mode.
>>>>     Re-base.
>>>> v2: Re-base over NOP addition to testcases modified here. Add IN/OUT
>>>>     tests. Add shift/rotate insn single operand tests. Extend
>>>>     "Instruction Naming" doc section to spell out defaults.
>>>
>>> Since we don't change encoding, we shouldn't drop any tests.  If a warning is
>>> a problem, we need to create new tests to silence the warning.
>>
>> Could you give a concrete example of a truly dropped test? Some pieces
>> get moved, but I view this as fulfilling "create new tests to silence
>> the warning".
> 
> The patch is OK.

Thanks much! Considering the impending branching of 2.34, personally
I'd think it would be better for me to delay committing, such that
this pretty significant change will get some more lingering time
before actually hitting a release. What do you think? (And of course
it would also seem better if some more of this series went in closely
together with this patch.)

Jan