Re: [PATCH v8 2/2] x86/AT&T: don't default to byte source for ambiguous for MOVSX/MOVZX

[Jan Beulich <jbeulich at suse dot com>]

On 17.02.2020 18:18, H.J. Lu wrote:
> On Mon, Feb 17, 2020 at 9:04 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 17.02.2020 17:55, H.J. Lu wrote:
>>> On Mon, Feb 17, 2020 at 7:37 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 17.02.2020 15:14, H.J. Lu wrote:
>>>>> On Mon, Feb 17, 2020 at 5:44 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>
>>>>>> On 14.02.2020 17:51, H.J. Lu wrote:
>>>>>>> On Fri, Feb 14, 2020 at 8:35 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>>>
>>>>>>>> On 14.02.2020 17:26, H.J. Lu wrote:
>>>>>>>>> On Fri, Feb 14, 2020 at 7:52 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>>>>>
>>>>>>>>>> On 14.02.2020 15:37, H.J. Lu wrote:
>>>>>>>>>>> On Fri, Feb 14, 2020 at 6:32 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> On 14.02.2020 15:25, H.J. Lu wrote:
>>>>>>>>>>>>> On Fri, Feb 14, 2020 at 6:23 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 14.02.2020 15:16, H.J. Lu wrote:
>>>>>>>>>>>>>>> On Fri, Feb 14, 2020 at 5:54 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 14.02.2020 13:28, H.J. Lu wrote:
>>>>>>>>>>>>>>>>> On Fri, Feb 14, 2020 at 4:26 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> As pointed out in "x86: replace adhoc (partly wrong) ambiguous operand
>>>>>>>>>>>>>>>>>> checking for MOVSX/MOVZX" silently guessing what the programmer may have
>>>>>>>>>>>>>>>>>> meant is not helpful, the more that we don't do so elsewhere anymore
>>>>>>>>>>>>>>>>>> (except in cases where it is overwhelmingly likely that the other case
>>>>>>>>>>>>>>>>>> isn't meant, like here for it meant to be a "sign/zero extension" from
>>>>>>>>>>>>>>>>>> 16 bits to 16 bits).
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> gas/
>>>>>>>>>>>>>>>>>> 2020-02-XX  Jan Beulich  <jbeulich@suse.com>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>         PR gas/25438
>>>>>>>>>>>>>>>>>>         * config/tc-i386.c (process_suffix): Default movsx/movzx to byte
>>>>>>>>>>>>>>>>>>         suffix only when destination is a word reg.
>>>>>>>>>>>>>>>>>>         testsuite/gas/i386/noreg16.l, testsuite/gas/i386/noreg32.l,
>>>>>>>>>>>>>>>>>>         testsuite/gas/i386/noreg64.l: Adjust expectations.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> No need for this since this is documented behavior of AT&T syntax.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I've just looked at the documentation again: As said in the
>>>>>>>>>>>>>>>> other reply to your doc change, these mnemonics aren't
>>>>>>>>>>>>>>>> mentioned as legal in Solaris'es AT&T spec. And I also
>>>>>>>>>>>>>>>> can't find gas doc saying so. Would you please point me at
>>>>>>>>>>>>>>>> where this is being documented?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Solaris spec doesn't mention movsx[bwl] nor movsx.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Right, so where did you take from that "this is documented behavior"?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Documented in gas manual.
>>>>>>>>>>>>
>>>>>>>>>>>> Where? As said, I did look there without finding anything to this effect.
>>>>>>>>>>>
>>>>>>>>>>> It is there now.
>>>>>>>>>>
>>>>>>>>>> Hmm, even worse: It's not there. Neither of the two doc changes of
>>>>>>>>>> yours today say anything like this. Did you forget to both post a
>>>>>>>>>> patch _and_ push it, or am I blind, or are you simply lying to me?
>>>>>>>>>> I'm sorry for being blunt, but I'm getting really annoyed.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> info as.info on master branch shows
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>  The Intel-syntax extension instructions
>>>>>>>>>
>>>>>>>>>    * 'movsx' -- sign-extend 'reg8/mem8' to 'reg16'.
>>>>>>>>>
>>>>>>>>>    * 'movsx' -- sign-extend 'reg8/mem8' to 'reg32'.
>>>>>>>>>
>>>>>>>>>    * 'movsx' -- sign-extend 'reg8/mem8' to 'reg64' (x86-64 only).
>>>>>>>>>
>>>>>>>>>    * 'movsx' -- sign-extend 'reg16/mem16' to 'reg32'
>>>>>>>>>
>>>>>>>>>    * 'movsx' -- sign-extend 'reg16/mem16' to 'reg64' (x86-64 only).
>>>>>>>>>
>>>>>>>>>    * 'movsxd' -- sign-extend 'reg32/mem32' to 'reg64' (x86-64 only).
>>>>>>>>>
>>>>>>>>>    * 'movzx' -- zero-extend 'reg8/mem8' to 'reg16'.
>>>>>>>>>
>>>>>>>>>    * 'movzx' -- zero-extend 'reg8/mem8' to 'reg32'.
>>>>>>>>>
>>>>>>>>>    * 'movzx' -- zero-extend 'reg8/mem8' to 'reg64' (x86-64 only).
>>>>>>>>>
>>>>>>>>>    * 'movzx' -- zero-extend 'reg16/mem16' to 'reg32'
>>>>>>>>>
>>>>>>>>>    * 'movzx' -- zero-extend 'reg16/mem16' to 'reg64' (x86-64 only).
>>>>>>>>>
>>>>>>>>> are called 'movsbw/movsxb/movsx', 'movsbl/movsxb/movsx',
>>>>>>>>> 'movsbq/movsb/movsx', 'movswl/movsxw', 'movswq/movsxw', 'movslq/movsxl',
>>>>>>>>> 'movzbw/movzxb/movzx', 'movzbl/movzxb/movzx', 'movzbq/movzxb/movzx',
>>>>>>>>> 'movzwl/movzxw' and 'movzwq/movzxw' in AT&T syntax.
>>>>>>>>
>>>>>>>> Which doesn't say anywhere that with a memory operand but
>>>>>>>> without suffix both default to a byte source. Yet that's the
>>>>>>>> whole point of the discussion we're having.
>>>>>>>>
>>>>>>>
>>>>>>> Does 'movsbw/movsxb/movsx' cover it?
>>>>>>
>>>>>> No, it doesn't, because its companion 'movswl/movsxw' is obviously
>>>>>> (to me at least) incomplete, as for register operands plain "movsx"
>>>>>> is suitable too. It's imo wrong in the first place to formally
>>>>>> name any movsx* or movzx* mnemonics to be valid in AT&T syntax. As
>>>>>> said, the Solaris spec doesn't mention them. Hence they're at best
>>>>>> hybrids, which we mean to permit, but without making them legal AT&T
>>>>>> syntax.
>>>>>
>>>>> Not document movsxX/movsx is fine.  But we should just silently accept
>>>>> them since assembly codes are encoded correctly.
>>>>
>>>> We should accept them, yes, _but not silently_. Even less so without
>>>> it being written down anywhere that this is the behavior to expect.
>>>
>>> Please submit a patch to improve gas manual to document such.
>>
>> Just like on the other thread - I'm not going to document what I
>> believe is wrong behavior. If I am to add documentation here, I'd
>> add it such that afterwards the implementation will need bringing
>> in sync. Also, let's not forget - you objected to a code change
>> mine on the basis that the way you wanted it is documented
>> _somewhere_. And now you ask _me_ to add this documentation?
> 
> I did add documentation.

You mean the part still quoted above? As you were able to see, even
being aware of the context I couldn't interpret it as even coming
close to what you meant to document. It does in no way support your
claim of there being a defaulting to byte source. (And of course it
also continues to be the case that you claimed such documentation
was in place _before_ you actually added this piece.) I don't think
I'm in the slightest convinced that suppressing the warning in this
particular case is warranted. In fact the process of the discussion
has convinced me even further that a warning _is needed_ (and not
just warranted) here.

Jan