public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: "H.J. Lu" <hjl.tools@gmail.com>
Cc: "binutils@sourceware.org" <binutils@sourceware.org>
Subject: Re: [PATCH v8 2/2] x86/AT&T: don't default to byte source for ambiguous for MOVSX/MOVZX
Date: Mon, 17 Feb 2020 15:37:00 -0000	[thread overview]
Message-ID: <b2017a00-32fb-6f82-5ac4-b7a48cc5e02c@suse.com> (raw)
In-Reply-To: <CAMe9rOoF=Lkmyah9cz_rDNexhT12vSZpiNRhNgG=AuAOjtP2qg@mail.gmail.com>

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.

Jan

  reply	other threads:[~2020-02-17 15:37 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-14 12:21 [PATCH v8 0/2] x86: replace adhoc (partly wrong) ambiguous operand checking " Jan Beulich
2020-02-14 12:26 ` [PATCH v8 2/2] x86/AT&T: don't default to byte source for ambiguous " Jan Beulich
2020-02-14 12:28   ` H.J. Lu
2020-02-14 13:13     ` Jan Beulich
2020-02-14 13:54     ` Jan Beulich
2020-02-14 14:16       ` H.J. Lu
2020-02-14 14:23         ` Jan Beulich
2020-02-14 14:26           ` H.J. Lu
2020-02-14 14:32             ` Jan Beulich
2020-02-14 14:38               ` H.J. Lu
2020-02-14 15:47                 ` Jan Beulich
2020-02-14 15:52                 ` Jan Beulich
2020-02-14 16:26                   ` H.J. Lu
2020-02-14 16:36                     ` Jan Beulich
2020-02-14 16:52                       ` H.J. Lu
2020-02-17 13:44                         ` Jan Beulich
2020-02-17 14:14                           ` H.J. Lu
2020-02-17 15:37                             ` Jan Beulich [this message]
2020-02-17 16:56                               ` H.J. Lu
2020-02-17 17:04                                 ` Jan Beulich
2020-02-17 17:19                                   ` H.J. Lu
2020-02-18  7:44                                     ` Jan Beulich
2020-02-14 12:26 ` [PATCH v8 1/2] x86: replace adhoc (partly wrong) ambiguous operand checking " Jan Beulich
2020-02-14 12:31   ` H.J. Lu
2020-02-14 12:34   ` H.J. Lu
2020-02-14 12:42     ` Jan Beulich
2020-02-14 12:49       ` H.J. Lu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b2017a00-32fb-6f82-5ac4-b7a48cc5e02c@suse.com \
    --to=jbeulich@suse.com \
    --cc=binutils@sourceware.org \
    --cc=hjl.tools@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).