> Date: Wed, 19 Apr 2023 22:16:36 +0200 > From: Bernhard Reutner-Fischer <rep.dot....@gmail.com>
> 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? (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