Re: [PATCH v5 2/5] x86: move certain MOVSX/MOVZX tests

[Jan Beulich <jbeulich at suse dot com>]

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
please also only on top of my changes, so I won't need to re-base
_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