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

[Jan Beulich <jbeulich@suse.com>]

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.)

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