Re: [PATCH] Replace IgnoreSize/DefaultSize with MnemonicSize

[Jan Beulich <jbeulich@suse.com>]

On 03.03.2020 18:15, H.J. Lu wrote:
> On Tue, Mar 3, 2020 at 6:50 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 03.03.2020 15:09, H.J. Lu wrote:
>>> I am testing this patch with GCC 8.  I will check it in if it fixes
>>> regressions in GCC 8 testsuits:
>>>
>>> https://gcc.gnu.org/ml/gcc-regression/2020-03/msg00008.html
>>>
>>> H.J.
>>> ---
>>> According to gas manual, suffix in instruction mnemonics isn't always
>>> required:
>>>
>>> When there is no sizing suffix and no (suitable) register operands to
>>> deduce the size of memory operands, with a few exceptions and where long
>>> operand size is possible in the first place, operand size will default
>>> to long in 32- and 64-bit modes.
>>
>> Nothing there says that this defaulting is to happen silently. Yet
>> _that's_ what my earlier changes altered. The defaulting is still
>> the same. And no - SUCH CASES SHOULD NOT GO SILENTLY, neither here
>> nor in the MOVSX/MOVZX case. Ambiguities should _always_ be
>> pointed out by the assembler. (There may be [and there is] a mode
>> in which this goes silently, to be enabled at the programmer's
>> risk.)
> 
> It is not going to happen in AT&T syntax.   Gas has to support older GCC
> without any warnings.

Why? What's wrong with pointing out issues even with compiler
generated code? In fact iirc gcc used to be buggy in regard of
these conversion instructions, and the assembler change helped
spot this.

>>> This includes cvtsi2sd, cvtsi2ss, vcvtsi2sd, vcvtsi2ss, vcvtusi2sd and
>>> vcvtusi2ss.  Since they are used in GCC 8 and older GCC releases, they
>>> must be allowed without suffix in AT&T syntax.
>>>
>>> gas/
>>>
>>>       PR gas/25622
>>>       * testsuite/gas/i386/i386.exp: Run x86-64-default-suffix and
>>>       x86-64-default-suffix-avx.
>>>       * testsuite/gas/i386/noreg64.s: Remove cvtsi2sd, cvtsi2ss,
>>>       vcvtsi2sd, vcvtsi2ss, vcvtusi2sd and vcvtusi2ss entries.
>>>       * testsuite/gas/i386/noreg64.d: Updated.
>>>       * testsuite/gas/i386/noreg64.l: Likewise.
>>>       * testsuite/gas/i386/x86-64-default-suffix-avx.d: New file.
>>>       * testsuite/gas/i386/x86-64-default-suffix.d: Likewise.
>>>       * testsuite/gas/i386/x86-64-default-suffix.s: Likewise.
>>>
>>> opcodes/
>>>
>>>       PR gas/25622
>>>       * i386-opc.tbl: Add IgnoreSize to cvtsi2sd, cvtsi2ss, vcvtsi2sd,
>>>       vcvtsi2ss, vcvtusi2sd and vcvtusi2ss for AT&T syntax.
>>
>> Oh no. I'm trying to clean up the IgnoreSize mess and you want to
>> add new instances for no good reason (yes, there are cases where
>> this is actually missing; hopefully I'll get to send out the
>> series later this week).
> 
> Since an instruction template can't have both IgnoreSize and DefaultSize,
> I am testing this patch and will check it if there are no regressions.  Then
> we can add one value to MnemonicSize.

It seems pretty unrelated here, but is a good change to make,
I think.

>> I know I can't prevent this going in, but I'm heavily opposed.
>> You don't "fix" anything here, you break things.
> 
> I disagree.

It's pretty sad that in binutils consensus isn't required for
changes to go in. I'll enter a bug in due course in any event.

Jan