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 v5 2/5] x86: move certain MOVSX/MOVZX tests
Date: Tue, 11 Feb 2020 13:04:00 -0000	[thread overview]
Message-ID: <fdc215b8-8f77-7b85-675c-5a07d8bf0532@suse.com> (raw)
In-Reply-To: <CAMe9rOqpXY1bKAGnECRght5d+mUZtiK3GtV6o4Ka5FXLKDZ8QA@mail.gmail.com>

On 11.02.2020 14:01, H.J. Lu wrote:
> On Tue, Feb 11, 2020 at 4:58 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 11.02.2020 13:19, H.J. Lu wrote:
>>> On Tue, Feb 11, 2020 at 3:55 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 11.02.2020 12:42, H.J. Lu wrote:
>>>>> On Tue, Feb 11, 2020 at 2:25 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>
>>>>>> Some encodings are about to gain a warning - move them from test cases
>>>>>> not expecting any diagnostics to the new, dedicated ones, to allow
>>>>>> better focus on the actual changes in the subsequent patch.
>>>>>>
>>>>>> The new tests added have some wrong expectations right now, which will
>>>>>> be corrected by the next patch. The test is being added here to make
>>>>>> more visible which cases actually were wrong (and hence get changed),
>>>>>> besides demonstrating that in the vast majority of cases the subsequent
>>>>>> change doesn't alter generated code.
>>>>>>
>>>>>> gas/
>>>>>> 2020-02-XX  Jan Beulich  <jbeulich@suse.com>
>>>>>>
>>>>>>         * testsuite/gas/i386/i386.s, testsuite/gas/i386/iamcu-1.s,
>>>>>>         testsuite/gas/i386/ilp32/x86-64.s: Move ambiguous operand size
>>>>>>         tests ...
>>>>>>         * testsuite/gas/i386/noreg16.s, testsuite/gas/i386/noreg32.s,
>>>>>>         testsuite/gas/i386/noreg64.s, testsuite/gas/i386/x86_64.s: ...
>>>>>>         here.
>>>>>>         * testsuite/gas/i386/i386.d, testsuite/gas/i386/i386-intel.d
>>>>>>         testsuite/gas/i386/iamcu-1.d, testsuite/gas/i386/ilp32/x86-64.d,
>>>>>>         testsuite/gas/i386/k1om.d, testsuite/gas/i386/l1om.d,
>>>>>>         testsuite/gas/i386/noreg16.d, testsuite/gas/i386/noreg32.d,
>>>>>>         testsuite/gas/i386/noreg64.d, testsuite/gas/i386/x86_64-intel.d,
>>>>>>         testsuite/gas/i386/x86_64.d: Adjust expectations.
>>>>>>         * testsuite/gas/i386/movx16.s, testsuite/gas/i386/movx16.l,
>>>>>>         testsuite/gas/i386/movx32.s, testsuite/gas/i386/movx32.l,
>>>>>>         testsuite/gas/i386/movx64.s, testsuite/gas/i386/movx64.l: New.
>>>>>>         * testsuite/gas/i386/i386.exp: Run new tests.
>>>>>
>>>>> Please make a separate patch to address MOVSX/MOVZX.
>>>>
>>>> I don't understand what you mean here. This patch simply documents the
>>>> status quo, to make it (much) easier to see what the next patch
>>>> actually adjusts. It doesn't "address" anything. If, for the purpose
>>>> of committing, you'd like to see both patches folded - fine by me. But
>>>> only then, not any earlier.
>>>>
>>>>>  MOVSX and MOVZX
>>>>> should take no suffixes.  AT&T syntax is supported if there is no
>>>>> ambiguity.  AT&T
>>>>> syntax also supports movsXY and movzXY.
>>>>
>>>> Please could you clarify what specifically you'd like to see changed,
>>>> at the very least by pointing out one case each where you think I'm
>>>> moving in the wrong direction (presumably in the next patch really)?
>>>> I'm afraid your response isn't such that I can derive from it what
>>>> exactly you want.
>>>
>>> We support
>>>
>>> movsx %ax, %ecx
>>> movzx %ax, %ecx
>>> movswl %ax, %ecx
>>> movzwl %ax, %ecx
>>>
>>> We disallow
>>>
>>> movsxw %ax, %ecx
>>> movzxw %ax, %ecx
>>
>> We don't (as this patch demonstrates, along with pre-existing tests,
>> unless you mean once again to have an inconsistency between insns
>> with all register operands and similar ones with e memory source),
>> and if you want it to be this way, then please do so yourself, but
> 
> I will do it.
> 
>> please also only on top of my changes, so I won't need to re-base
> 
> Which changes of yours are you referring to?

This patch and the subsequent one.

Jan

>> _yet_ another time.
>>
>> Just to repeat my request from an earlier version: Please take the
>> time to check what this patch does (documenting _just_ current
>> behavior), and what the next patch changes behavior-wise. And
>> please comment on that following patch in case you think it makes
>> a change that it shouldn't make, i.e. in particular one which
>> isn't in line with other similar behavior.
>>
>> Jan
> 
> 
> 

  reply	other threads:[~2020-02-11 13:04 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-11 10:23 [PATCH v5 0/5] x86: operand size handling improvements Jan Beulich
2020-02-11 10:25 ` [PATCH v5 2/5] x86: move certain MOVSX/MOVZX tests Jan Beulich
2020-02-11 11:43   ` H.J. Lu
2020-02-11 11:55     ` Jan Beulich
2020-02-11 12:20       ` H.J. Lu
2020-02-11 12:58         ` Jan Beulich
2020-02-11 13:02           ` H.J. Lu
2020-02-11 13:04             ` Jan Beulich [this message]
2020-02-11 13:07               ` H.J. Lu
2020-02-11 16:45                 ` Jan Beulich
2020-02-11 17:04                   ` H.J. Lu
2020-02-11 20:12                     ` [PATCH] x86: Remove movsx/movzx with memory operand from AT&T syntax H.J. Lu
2020-02-11 23:34                       ` H.J. Lu
2020-02-11 23:52                         ` H.J. Lu
2020-02-12  3:19                           ` [PATCH] x86: Remove movsx/movzx with 16/32-bit " H.J. Lu
2020-02-12  9:19                             ` Jan Beulich
2020-02-11 10:25 ` [PATCH v5 3/5] x86: replace adhoc (partly wrong) ambiguous operand checking for MOVSX/MOVZX Jan Beulich
2020-02-11 10:25 ` [PATCH v5 1/5] x86: also disallow non-byte/-word registers with byte/word suffix Jan Beulich
2020-02-11 11:27   ` H.J. Lu
2020-02-11 10:26 ` [PATCH v5 4/5] x86: correct VFPCLASSP{S,D} operand size handling Jan Beulich
2020-02-11 11:50   ` H.J. Lu
2020-02-11 12:49     ` Jan Beulich
2020-02-11 12:56       ` H.J. Lu
2020-02-11 10:27 ` [PATCH v5 5/5] x86-64: Intel64 adjustments for insns dealing with far pointers Jan Beulich
2020-02-11 11:53   ` H.J. Lu
2020-02-12  8:11     ` Jan Beulich

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=fdc215b8-8f77-7b85-675c-5a07d8bf0532@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).