Re: [PATCH v3 4/7] x86-64: further re-work insn/suffix recognition to also cover MOVSL

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

On Tue, Oct 18, 2022 at 11:08 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 18.10.2022 23:48, H.J. Lu wrote:
> > On Mon, Oct 17, 2022 at 11:31 PM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 18.10.2022 00:36, H.J. Lu wrote:
> >>> On Mon, Oct 17, 2022 at 12:02 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> On 14.10.2022 19:07, H.J. Lu wrote:
> >>>>> On Fri, Oct 14, 2022 at 12:03 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>
> >>>>>> On 13.10.2022 19:00, H.J. Lu wrote:
> >>>>>>> On Wed, Oct 12, 2022 at 11:08 PM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>>>
> >>>>>>>> On 12.10.2022 19:10, H.J. Lu wrote:
> >>>>>>>>> On Wed, Oct 12, 2022 at 12:08 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>>>>>
> >>>>>>>>>> On 11.10.2022 19:44, H.J. Lu wrote:
> >>>>>>>>>>> On Wed, Oct 5, 2022 at 12:24 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>>>>>>>
> >>>>>>>>>>>> PR gas/29524
> >>>>>>>>>>>> In order to make MOVSL{,Q} behave similarly to MOVSB{W,L,Q} and
> >>>>>>>>>>>> MOVSW{L,Q} we need to defer parse_insn()'s emitting of errors unrelated
> >>>>>>>>>>>> to prefix parsing. Utilize i.error just like match_template() does.
> >>>>>>>>>>>
> >>>>>>>>>>> Since movs{b,w,l,q} are string instructions, integer sign extensions
> >>>>>>>>>>> require a suffix to specify the destination size.  This is different from other
> >>>>>>>>>>> integer instructions.  Since only the new assembler allows the implicit suffix,
> >>>>>>>>>>> it won't be easy to use.  We should improve error messages, but allowing
> >>>>>>>>>>> new syntax doesn't help much.
> >>>>>>>>>>
> >>>>>>>>>> It is an earlier change making most of this consistent with MOVZ*; it is
> >>>>>>>>>
> >>>>>>>>> MOVZ is different.  There are no MOVZ string instructions.  MOVS has
> >>>>>>>>> different meanings in ISA.   MOVS difference from MOVZ in assembly
> >>>>>>>>> syntax should be expected.
> >>>>>>>>
> >>>>>>>> You've said so before, yes, but I continue to disagree. And as we can see
> >>>>>>>> from the series things can be made work consistently (and imo nothing else
> >>>>>>>> should have been done right from the beginning).
> >>>>>>>>
> >>>>>>>
> >>>>>>> There are inconsistencies in ISA.
> >>>>>>
> >>>>>> Sure. But we shouldn't add further ones in the assembler.
> >>>>>
> >>>>> Assembler just follows ISA.  Programmers should learn to
> >>>>> deal with it or use a compiler.
> >>>>
> >>>> This is entirely non-constructive. Assembler writers should get things into
> >>>> usable (read: consistent) shape. Plus what ISA are you talking about here?
> >>>
> >>> GNU assembler has been this way for a long time and the current GNU
> >>> assembler will still be in use for the next few years.  Assembler writers
> >>> should know about all these.
> >>
> >> Hmm, so after all not any ISA to follow? Plus do you suggest there's
> >> only people having written x86 assembler code for many years? And
> >> there's no people who would prefer to get their code into more
> >> consistent shape, but who are limited by assembler shortcomings?
> >
> > I prefer consistency with existing assemblers and ISA specs.
> > For new instructions, suffixes should be used only when there is
> > an ambiguity.  For existing instructions, to support existing assembly
> > codes, suffixes may be optional even when there is an ambiguity.
> > But for integer MOVS instructions, suffixes have always been required.
> > I don't think we should change them now.  We can improve documentation
> > if needed.
> >
> >> In any event, ...
> >>
> >>>> We're talking of mnemonics which aren't spelled out in any ISA document
> >>>> anyway. The only halfway official AT&T doc I'm aware of doesn't provide
> >>>> room for omitting size suffixes [1]. Yet that's a fundamental feature of gas,
> >>>> and elsewhere (recently: CMPccXADD) you're even suggesting to force people
> >>>> to omit suffixes (plus you've previously objected to the disassembler to
> >>>> consistently emit them in suffix-always mode).
> >>>>
> >>>> That same document was also only updated to cover 64-bit code in a half-
> >>>> hearted way, so can't necessarily be used for 64-bit only insns (it doesn't
> >>>> list any form of MOVSXD at all afaics, for example). Where not explicitly
> >>>> mentioned, their intended handling can only be inferred by using analogies.
> >>>> Nor do we support some of the odd (quirky I would say) mnemonics that are
> >>>> listed there, like xchglA or movabsbA (which is even wrongly described as
> >>>> moving an immediate value into the register).
> >>>>
> >>>> Bottom line: May I please ask that you take another (constructive) look at
> >>>> the v4 submission?
> >>
> >> ... this request continues to stand.
> >
> > I think we should improve diagnostics and documentation, not add new
> > syntaxes to existing instructions.
>
> I continue to disagree, but to make at least some forward progress: Which
> parts of this series can I expect an okay for (patches 5-7 already have
> one, but can't go in ahead)? I would try to re-order the series then to
> put the controversial patch(es) at the end, such that at least parts can
> go in and I can make further progress with other work. But there's no
> promise this re-ordering actually is going to work out if it's more than
> just this one patch which you continue to object to.
>

Please submit a new patch set without MOVS changes.

Thanks.

-- 
H.J.