Re: [PATCH v6 2/7] x86: re-work insn/suffix recognition

[Jan Beulich <jbeulich@suse.com>]

On 08.11.2022 02:21, H.J. Lu wrote:
> On Mon, Nov 7, 2022 at 2:23 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 05.11.2022 00:54, H.J. Lu wrote:
>>> On Fri, Nov 4, 2022 at 3:51 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> Having templates with a suffix explicitly present has always been
>>>> quirky. Introduce a 2nd matching pass in case the 1st one couldn't find
>>>> a suitable template _and_ didn't itself already need to trim off a
>>>> suffix to find a match at all. This requires error reporting adjustments
>>>> (albeit luckily fewer than I was afraid might be necessary), as errors
>>>> previously reported during matching now need deferring until after the
>>>> 2nd pass (because, obviously, we must not emit any error if the 2nd pass
>>>> succeeds). While also related to PR gas/29524, it was requested that
>>>> move-with-sign-extend be left as broken as it always was.
>>>>
>>>> PR gas/29525
>>>> Note that with the dropped CMPSD and MOVSD Intel Syntax string insn
>>>> templates taking operands, mixed IsString/non-IsString template groups
>>>> (with memory operands) cannot occur anymore. With that
>>>> maybe_adjust_templates() becomes unnecessary (and is hence being
>>>> removed).
>>>>
>>>> PR gas/29526
>>>> Note further that while the additions to the intel16 testcase aren't
>>>> really proper Intel syntax, we've been permitting all of those except
>>>> for the MOVD variant. The test therefore is to avoid re-introducing such
>>>> an inconsistency.
>>>> ---
>>>> To limit code churn I'm using "goto" for the retry loop, but I'd be
>>>> happy to make this a proper loop either right here or in a follow-on
>>>> change doing just the necessary re-indentation.
>>>>
>>>> The "too many memory references" errors which are being deleted weren't
>>>> fully consistent anyway - even the majority of IsString insns accepts
>>>> only a single memory operand. If we wanted to retain that, it would need
>>>> re-introducing in md_assemble(), latching the error into i.error just
>>>> like match_template() does.
>>>>
>>>> As an unrelated observation: Why is "MOVQ $imm64, %reg64" being
>>>> optimized but "MOVABS $imm64, %reg64" is not?
>>>> ---
>>>> v6: Re-base over dropping of Pass2 attribute.
>>>> v5: Split off move-with-sign-extend changes.
>>>> v4: Retain support for operand-less MOVSD and CMPSD.
>>>> v3: Limit xstrdup() to just the templates where a 2nd pass actually
>>>>     makes sense (new Pass2 attribute).
>>>
>>> I don't think we should add a second pass.
>>
>> So you've asked me to re-work the series several times just to _now_
>> say "no" altogether? What's your alternative proposal to address the
>> various shortcomings that this series is addressing? (Yes, patches 4
>> and 5 can, with some effort, probably be re-based ahead, but those
>> are only minor improvements found while doing the main piece of work
>> here, and they aren't strictly related to the main goal of the series.)
> 
> The more I looked, the more I didn't think we needed the second pass.
> The issues you raised are minor issues.  We shouldn't add the second
> pass because of them.

IOW your view is "let's keep the assembler broken, and let's keep
surprising assembly programmers". I'm afraid what is to be considered
"minor" is up for debate _and_ it is also up for debate whether leaving
"minor" issues unfixed is okay. The more that I've already done the
work in the cases here. Please revisit your looking at it.

Furthermore I don't think patch 3 (which is merely a first step to take)
can really fall in that "minor" category: The "inventing" of suffixes
in Intel Syntax mode has always been bogus. Yet this patch builds on top
of what patch 2 does; at least at the moment I can't see doing that same
work without first removing all the stray "movq" templates. (Their
presence also gets in the way of patch 4, but there this could be dealt
with afaict.) In this context please also remember that I'm the
maintainer for Intel Syntax code, so it is at least an edge case for
you to block me making improvements there.

Nick, may I ask for a more project-wide view on this perspective that
H.J. is taking? I certainly don't mean to (ab)use my global maintainer
role to override him, but I also don't really agree with my view being
put off. Yet if there was a more general (unwritten) policy like this,
then of course I'd need to accept things the way they are.

Jan

>> Plus I now really feel urged to point out that you're blocking further
>> work I have pending, which I keep re-basing over all the adjustments I
>> was making to address your comments (plus of course the new ISA
>> extension patches which have gone in recently, all of which also
>> collide with work I'm doing). This re-basing is non-trivial and hence
>> is consuming a considerable amount of time as well.
>>
>> Jan
> 
> 
>