Re: [PATCH v3] doc: Document order of define_peephole2 scanning

[Hans-Peter Nilsson <hp@axis.com>]

> Date: Wed, 19 Apr 2023 22:16:36 +0200
> From: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>

> On 19 April 2023 21:21:18 CEST, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
> >Hi H-P!
> >
> >This begs the question iff now (i fear it's not), upon
> >successful match(es), the whole peepholes get re-run
> >again per BB (ATM?), exposing more opportunities?

(Not sure IIUC the question, but:) no; as mentioned the
peephole2 machinery doesn't back up to include the context
of longer sequences when having done replacement for a
shorter match.  It resumes matching at the beginning of the
shorter, matched and replaced, sequence.

> >If not, would we want to retry, at least for
> >-fexpensive-optimisations (sp?) or would all hell break
> >loose?

I don't think hell would break loose, but maybe slowdown
and/or buggy define_peephole2s would be weeded out.  Or
something else entirely unexpected. :)

> >Please also see below.
> >
> >On 19 April 2023 18:59:14 CEST, Hans-Peter Nilsson via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> >>Anyway, the missing-context problem I ran into remains: if
> >>you have an insn sequence {foo bar} and a define_peephole2
> >>matching and replacing {bar} into {baz}, the resulting {foo
> >>baz} *will not be matched* against a define_peephole2
> >>looking for {foo baz}.  But, I'm not trying to document this
> >>caveat specifically, though at least it'll now be implied by
> >>the documentation.
> >>
> >>This could be fixed by always backing up MAX_INSNS_PER_PEEP2
> >>- 1 insns after a successful replacement.  I'm somewhat
> >>worries that this would also mean lots of futile re-match
> >>attempts.  Thoughts?
> >
> >Good point. Probably. Do you have stats?

None whatsoever.  I'm going to test with the "original"
define_peephole2 for CRIS that was suffering, but with an
"extra" define_peephole2 that also does the modification for
the one that caused it do be missed and see if the original
free one still fires off.  (I see I cut down my foo bar baz
examples too much to make sense to explain that.)

> >If there is even a slight benefit, then some certainly
> >would be willing to take that penalty for sure. I.e. iff
> >it helps -Os or locality then such expensive
> >optimisations certainly provide benefit for at least a
> >few if not some, IMO.

Right.  Not sure I'll be doing it though, having other
things, gcc-related and others, on my plate.  If so, first
I'd do a crude attempt at getting statistics for x86_64, by
after a successful replacement, restarting at the beginning
of a BB and checking whether any define_peephole2 fires off
before reaching the first replaced insn.

brgds, H-P