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

["H.J. Lu" <hjl.tools@gmail.com>]

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?

> _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



-- 
H.J.