Re: [PATCH] doc: Document order of define_peephole2 scanning
> From: Paul Koning
> Date: Tue, 18 Apr 2023 14:32:07 -0400
>
> I'm not sure about the meaning of part of this.
> "...resumes at the last generated insn." Does that mean:
(Neither...)
> 1. If a match is found at some insn, the replacement
> defined by the matching define_peephole2 is performed, and
> then the scan resumes at the first of the insns produced
> by the replacement.
This was what I expected. If it had been this, I wouldn't
have suggested the doc update. But it isn't: no, it's the
*last produced one*. If you look at the referenced example
(unfortunately outside of the diff context) it should all be
clear.
> or
>
> 2. If a match is found at some insn, the replacement
> defined by the matching define_peephole2 is performed, and
> then the scan resumes at the insn immediately following
> the ones just matched.
No, from the last of the replacement insns.
> "Last generated" seems to fit option 1,
Sorry, your confusement confuses me. I just don't see how
to confuse last with first or matched with generated. :)
> but I'm not sure
> if that's what you meant. Maybe you could add some words
> to say more explicitly which it is.
I'm referring to an example on the same pdf page.
But perhaps s/resumes at the last generated insn/resumes at
the last insn in the replacement sequence/ would help?
brgds, H-P
>
> paul
>
> > On Apr 18, 2023, at 1:55 PM, Hans-Peter Nilsson via Gcc-patches
> > wrote:
> >
> > Generated pdf inspected. Ok to commit?
> >
> > Thoughts on fixing the IMHO wart to also expose all
> > replacements to all define_peephole2? Looks feasible
> > (famous last words), but then again I haven't checked the
> > history yet.
> >
> > -- >8 --
> > I was a bit surprised when my define_peephole2 didn't match,
> > but it was because it was expected to partially match the
> > generated output of a previous define_peephole2. I had
> > assumed that the algorithm exposed newly created opportunities
> > to all define_peephole2's. While things can change in that
> > direction, let's start with documenting the current state.
> >
> > * doc/md.texi (define_peephole2): Document order of scanning.
> > ---
> > gcc/doc/md.texi | 8
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
> > index 07bf8bdebffb..0f9e32d2c648 100644
> > --- a/gcc/doc/md.texi
> > +++ b/gcc/doc/md.texi
> > @@ -9362,6 +9362,14 @@ If the preparation falls through (invokes neither
> > @code{DONE} nor
> > @code{FAIL}), then the @code{define_peephole2} uses the replacement
> > template.
> >
> > +Insns are scanned in forward order from beginning to end for each basic
> > +block, but the basic blocks are scanned in reverse order of appearance
> > +in a function. After a successful replacement, scanning for further
> > +opportunities for @code{define_peephole2} matches, resumes at the last
> > +generated insn. I.e. for the example above, the first insn that can be
> > +matched by another @code{define_peephole2}, is @code{(set (match_dup 3)
> > +(match_dup 4))}.
> > +
> > @end ifset
> > @ifset INTERNALS
> > @node Insn Attributes
> > --
> > 2.30.2
> >
>
Re: [PATCH] doc: Document order of define_peephole2 scanning
I'm not sure about the meaning of part of this. "...resumes at the last
generated insn." Does that mean:
1. If a match is found at some insn, the replacement defined by the matching
define_peephole2 is performed, and then the scan resumes at the first of the
insns produced by the replacement.
or
2. If a match is found at some insn, the replacement defined by the matching
define_peephole2 is performed, and then the scan resumes at the insn
immediately following the ones just matched.
"Last generated" seems to fit option 1, but I'm not sure if that's what you
meant. Maybe you could add some words to say more explicitly which it is.
paul
> On Apr 18, 2023, at 1:55 PM, Hans-Peter Nilsson via Gcc-patches
> wrote:
>
> Generated pdf inspected. Ok to commit?
>
> Thoughts on fixing the IMHO wart to also expose all
> replacements to all define_peephole2? Looks feasible
> (famous last words), but then again I haven't checked the
> history yet.
>
> -- >8 --
> I was a bit surprised when my define_peephole2 didn't match,
> but it was because it was expected to partially match the
> generated output of a previous define_peephole2. I had
> assumed that the algorithm exposed newly created opportunities
> to all define_peephole2's. While things can change in that
> direction, let's start with documenting the current state.
>
> * doc/md.texi (define_peephole2): Document order of scanning.
> ---
> gcc/doc/md.texi | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
> index 07bf8bdebffb..0f9e32d2c648 100644
> --- a/gcc/doc/md.texi
> +++ b/gcc/doc/md.texi
> @@ -9362,6 +9362,14 @@ If the preparation falls through (invokes neither
> @code{DONE} nor
> @code{FAIL}), then the @code{define_peephole2} uses the replacement
> template.
>
> +Insns are scanned in forward order from beginning to end for each basic
> +block, but the basic blocks are scanned in reverse order of appearance
> +in a function. After a successful replacement, scanning for further
> +opportunities for @code{define_peephole2} matches, resumes at the last
> +generated insn. I.e. for the example above, the first insn that can be
> +matched by another @code{define_peephole2}, is @code{(set (match_dup 3)
> +(match_dup 4))}.
> +
> @end ifset
> @ifset INTERNALS
> @node Insn Attributes
> --
> 2.30.2
>
[PATCH] doc: Document order of define_peephole2 scanning
Generated pdf inspected. Ok to commit?
Thoughts on fixing the IMHO wart to also expose all
replacements to all define_peephole2? Looks feasible
(famous last words), but then again I haven't checked the
history yet.
-- >8 --
I was a bit surprised when my define_peephole2 didn't match,
but it was because it was expected to partially match the
generated output of a previous define_peephole2. I had
assumed that the algorithm exposed newly created opportunities
to all define_peephole2's. While things can change in that
direction, let's start with documenting the current state.
* doc/md.texi (define_peephole2): Document order of scanning.
---
gcc/doc/md.texi | 8
1 file changed, 8 insertions(+)
diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index 07bf8bdebffb..0f9e32d2c648 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -9362,6 +9362,14 @@ If the preparation falls through (invokes neither
@code{DONE} nor
@code{FAIL}), then the @code{define_peephole2} uses the replacement
template.
+Insns are scanned in forward order from beginning to end for each basic
+block, but the basic blocks are scanned in reverse order of appearance
+in a function. After a successful replacement, scanning for further
+opportunities for @code{define_peephole2} matches, resumes at the last
+generated insn. I.e. for the example above, the first insn that can be
+matched by another @code{define_peephole2}, is @code{(set (match_dup 3)
+(match_dup 4))}.
+
@end ifset
@ifset INTERNALS
@node Insn Attributes
--
2.30.2
