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

[Bernhard Reutner-Fischer via Gcc-patches]

[+list]
On 19 April 2023 21:21:18 CEST, Bernhard Reutner-Fischer 
<rep.dot....@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?
>If not, would we want to retry, at least for -fexpensive-optimisations (sp?) 
>or would all hell break loose?
>
>Please also see below.
>
>On 19 April 2023 18:59:14 CEST, Hans-Peter Nilsson via Gcc-patches 
><gcc-patches@gcc.gnu.org> wrote:
>>> From: Hans-Peter Nilsson <h...@axis.com>
>>> Date: Wed, 19 Apr 2023 06:06:27 +0200
>>> 
>>> Patch retracted, at least temporarily.  My "understanding"
>>> may be clouded by looking at an actual bug.  Sigh.
>>
>>Mea culpa.  I was looking at the result of one
>>define_peephole2 and thinking it was due to another, and
>>also tricked by incorrect code comments (patch posted, will
>>commit).
>>
>>TL;DR: Matching indeed does resume with attempting to match
>>the *first* define_peephole2 replacement insn.  But the
>>match-and-replacement order is largely undocumented.
>>
>>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?
>
>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.
>
>Just curious && cheers,
>
>>
>>(I could also just restart at the BB start, but I see all
>>this support for backing-up live info by single insns being
>>used.  Taking notes about a partial match for the first insn
>>of a failed attempt, as the maximum need to back-up to,
>>doesn't look like it'd fly, judging from the nonspecific
>>looking (set dest src) patterns being the first in i386
>>define_peephole2's match sequences.)
>