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

[Jan Beulich <jbeulich@suse.com>]

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.

Jan