public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] doc: Document order of define_peephole2 scanning
@ 2023-04-18 17:55 Hans-Peter Nilsson
  2023-04-18 18:32 ` Paul Koning
  0 siblings, 1 reply; 11+ messages in thread
From: Hans-Peter Nilsson @ 2023-04-18 17:55 UTC (permalink / raw)
  To: gcc-patches

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] doc: Document order of define_peephole2 scanning
  2023-04-18 17:55 [PATCH] doc: Document order of define_peephole2 scanning Hans-Peter Nilsson
@ 2023-04-18 18:32 ` Paul Koning
  2023-04-18 18:44   ` Hans-Peter Nilsson
  0 siblings, 1 reply; 11+ messages in thread
From: Paul Koning @ 2023-04-18 18:32 UTC (permalink / raw)
  To: Hans-Peter Nilsson, Hans-Peter Nilsson via Gcc-patches

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 <gcc-patches@gcc.gnu.org> 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
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] doc: Document order of define_peephole2 scanning
  2023-04-18 18:32 ` Paul Koning
@ 2023-04-18 18:44   ` Hans-Peter Nilsson
  2023-04-19  3:15     ` [PATCH v2] " Hans-Peter Nilsson
  0 siblings, 1 reply; 11+ messages in thread
From: Hans-Peter Nilsson @ 2023-04-18 18:44 UTC (permalink / raw)
  To: Paul Koning; +Cc: gcc-patches

> From: Paul Koning <paulkoning@comcast.net>

> 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 <gcc-patches@gcc.gnu.org> 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
> > 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2] doc: Document order of define_peephole2 scanning
  2023-04-18 18:44   ` Hans-Peter Nilsson
@ 2023-04-19  3:15     ` Hans-Peter Nilsson
  2023-04-19  4:06       ` Hans-Peter Nilsson
  0 siblings, 1 reply; 11+ messages in thread
From: Hans-Peter Nilsson @ 2023-04-19  3:15 UTC (permalink / raw)
  To: gcc-patches; +Cc: paulkoning

> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Tue, 18 Apr 2023 20:44:12 +0200
> 
> > From: Paul Koning <paulkoning@comcast.net>
> 
> > 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...)

[...]

> Sorry, your confusement confuses me.  I just don't see how
> to confuse last with first or matched with generated. :)

It's 4:30am and things appear much clearer, in particular
wrt. confusion.  Hopefully the version below is clearer.
Here's also the example from 35 lines up in md.texi:

(define_peephole2
  [(match_scratch:SI 4 "r")
   (set (match_operand:SI 0 "" "") (match_operand:SI 1 "" ""))
   (set (match_operand:SI 2 "" "") (match_dup 1))
   (match_dup 4)
   (set (match_operand:SI 3 "" "") (match_dup 1))]
  "/* @r{determine 1 does not overlap 0 and 2} */"
  [(set (match_dup 4) (match_dup 1))
   (set (match_dup 0) (match_dup 4))
   (set (match_dup 2) (match_dup 4))
   (set (match_dup 3) (match_dup 4))]
  "")

Approvers: pdf output reviewed.  Ok to commit?

All: thoughts on making define_peephole2 work "as expected";
"backtracing" so the replacement buffer ends with the first
generated replacement insn?  Might be simpler to restart at
the beginning of the BB, but I'm scared of overly long BB's.
Does anyone have statistics on the sizes of BB's in terms of
number of insns?

-- >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 backed-up the size of the match-buffer, thereby
exposing newly created opportunities with context 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..2ce043e6edc2 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.  Matches are attempted in order of appearance in the @file{md}
+file.  After a successful replacement, scanning for further
+opportunities for @code{define_peephole2}, resumes with the last
+generated replacement insn as the first insn to be matched.  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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] doc: Document order of define_peephole2 scanning
  2023-04-19  3:15     ` [PATCH v2] " Hans-Peter Nilsson
@ 2023-04-19  4:06       ` Hans-Peter Nilsson
  2023-04-19 16:59         ` [PATCH v3] " Hans-Peter Nilsson
  0 siblings, 1 reply; 11+ messages in thread
From: Hans-Peter Nilsson @ 2023-04-19  4:06 UTC (permalink / raw)
  To: gcc-patches; +Cc: paulkoning

> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Wed, 19 Apr 2023 05:15:27 +0200

> Approvers: pdf output reviewed.  Ok to commit?

Patch retracted, at least temporarily.  My "understanding"
may be clouded by looking at an actual bug.  Sigh.

brgds, H-P

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3] doc: Document order of define_peephole2 scanning
  2023-04-19  4:06       ` Hans-Peter Nilsson
@ 2023-04-19 16:59         ` Hans-Peter Nilsson
       [not found]           ` <4E63CF4D-C355-4053-8EA4-5700C1B938E3@gmail.com>
  2023-04-26 23:55           ` Ping: " Hans-Peter Nilsson
  0 siblings, 2 replies; 11+ messages in thread
From: Hans-Peter Nilsson @ 2023-04-19 16:59 UTC (permalink / raw)
  To: gcc-patches; +Cc: paulkoning

> From: Hans-Peter Nilsson <hp@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?

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

So again: Approvers: pdf output reviewed.  Ok to commit?
-- >8 --
I was a bit surprised when my newly-added define_peephole2 didn't
match, but it was because it was expected to partially match the
generated output of a previous define_peephole2, which matched and
modified the last insn of a sequence to be matched.  I had assumed
that the algorithm backed-up the size of the match-buffer, thereby
exposing newly created opportunities *with sufficient context* 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 | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index 07bf8bdebffb..300d104d58ab 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -9362,6 +9362,15 @@ 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.  Matches are attempted in order of @code{define_peephole2}
+appearance in the @file{md} file.  After a successful replacement,
+scanning for further opportunities for @code{define_peephole2}, resumes
+with the first generated replacement insn as the first insn to be
+matched against all @code{define_peephole2}.  For the example above,
+after its successful replacement, the first insn that can be matched by
+a @code{define_peephole2} is @code{(set (match_dup 4) (match_dup 1))}.
+
 @end ifset
 @ifset INTERNALS
 @node Insn Attributes
-- 
2.30.2


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3] doc: Document order of define_peephole2 scanning
       [not found]           ` <4E63CF4D-C355-4053-8EA4-5700C1B938E3@gmail.com>
@ 2023-04-19 20:16             ` Bernhard Reutner-Fischer
  2023-04-20  2:57               ` Hans-Peter Nilsson
  0 siblings, 1 reply; 11+ messages in thread
From: Bernhard Reutner-Fischer @ 2023-04-19 20:16 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: paulkoning, gcc-patches

[+list]
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?
>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 <hp@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.)
>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3] doc: Document order of define_peephole2 scanning
  2023-04-19 20:16             ` Bernhard Reutner-Fischer
@ 2023-04-20  2:57               ` Hans-Peter Nilsson
  0 siblings, 0 replies; 11+ messages in thread
From: Hans-Peter Nilsson @ 2023-04-20  2:57 UTC (permalink / raw)
  To: Bernhard Reutner-Fischer; +Cc: paulkoning, gcc-patches

> 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Ping: Re: [PATCH v3] doc: Document order of define_peephole2 scanning
  2023-04-19 16:59         ` [PATCH v3] " Hans-Peter Nilsson
       [not found]           ` <4E63CF4D-C355-4053-8EA4-5700C1B938E3@gmail.com>
@ 2023-04-26 23:55           ` Hans-Peter Nilsson
  2023-05-04  0:09             ` 2nd " Hans-Peter Nilsson
  1 sibling, 1 reply; 11+ messages in thread
From: Hans-Peter Nilsson @ 2023-04-26 23:55 UTC (permalink / raw)
  To: gcc-patches

> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Wed, 19 Apr 2023 18:59:14 +0200
[...]

> So again: Approvers: pdf output reviewed.  Ok to commit?
> -- >8 --
> I was a bit surprised when my newly-added define_peephole2 didn't
> match, but it was because it was expected to partially match the
> generated output of a previous define_peephole2, which matched and
> modified the last insn of a sequence to be matched.  I had assumed
> that the algorithm backed-up the size of the match-buffer, thereby
> exposing newly created opportunities *with sufficient context* 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 | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
> index 07bf8bdebffb..300d104d58ab 100644
> --- a/gcc/doc/md.texi
> +++ b/gcc/doc/md.texi
> @@ -9362,6 +9362,15 @@ 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.  Matches are attempted in order of @code{define_peephole2}
> +appearance in the @file{md} file.  After a successful replacement,
> +scanning for further opportunities for @code{define_peephole2}, resumes
> +with the first generated replacement insn as the first insn to be
> +matched against all @code{define_peephole2}.  For the example above,
> +after its successful replacement, the first insn that can be matched by
> +a @code{define_peephole2} is @code{(set (match_dup 4) (match_dup 1))}.
> +
>  @end ifset
>  @ifset INTERNALS
>  @node Insn Attributes
> -- 
> 2.30.2
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* 2nd Ping: Re: [PATCH v3] doc: Document order of define_peephole2 scanning
  2023-04-26 23:55           ` Ping: " Hans-Peter Nilsson
@ 2023-05-04  0:09             ` Hans-Peter Nilsson
  2023-05-04  9:45               ` Richard Biener
  0 siblings, 1 reply; 11+ messages in thread
From: Hans-Peter Nilsson @ 2023-05-04  0:09 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: gcc-patches

Ping again.

> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Thu, 27 Apr 2023 01:55:24 +0200
> 
> > From: Hans-Peter Nilsson <hp@axis.com>
> > Date: Wed, 19 Apr 2023 18:59:14 +0200
> [...]
> 
> > So again: Approvers: pdf output reviewed.  Ok to commit?
> > -- >8 --
> > I was a bit surprised when my newly-added define_peephole2 didn't
> > match, but it was because it was expected to partially match the
> > generated output of a previous define_peephole2, which matched and
> > modified the last insn of a sequence to be matched.  I had assumed
> > that the algorithm backed-up the size of the match-buffer, thereby
> > exposing newly created opportunities *with sufficient context* 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 | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
> > index 07bf8bdebffb..300d104d58ab 100644
> > --- a/gcc/doc/md.texi
> > +++ b/gcc/doc/md.texi
> > @@ -9362,6 +9362,15 @@ 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.  Matches are attempted in order of @code{define_peephole2}
> > +appearance in the @file{md} file.  After a successful replacement,
> > +scanning for further opportunities for @code{define_peephole2}, resumes
> > +with the first generated replacement insn as the first insn to be
> > +matched against all @code{define_peephole2}.  For the example above,
> > +after its successful replacement, the first insn that can be matched by
> > +a @code{define_peephole2} is @code{(set (match_dup 4) (match_dup 1))}.
> > +
> >  @end ifset
> >  @ifset INTERNALS
> >  @node Insn Attributes
> > -- 
> > 2.30.2
> > 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: 2nd Ping: Re: [PATCH v3] doc: Document order of define_peephole2 scanning
  2023-05-04  0:09             ` 2nd " Hans-Peter Nilsson
@ 2023-05-04  9:45               ` Richard Biener
  0 siblings, 0 replies; 11+ messages in thread
From: Richard Biener @ 2023-05-04  9:45 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: gcc-patches

On Thu, May 4, 2023 at 2:10 AM Hans-Peter Nilsson via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Ping again.

OK.

> > From: Hans-Peter Nilsson <hp@axis.com>
> > Date: Thu, 27 Apr 2023 01:55:24 +0200
> >
> > > From: Hans-Peter Nilsson <hp@axis.com>
> > > Date: Wed, 19 Apr 2023 18:59:14 +0200
> > [...]
> >
> > > So again: Approvers: pdf output reviewed.  Ok to commit?
> > > -- >8 --
> > > I was a bit surprised when my newly-added define_peephole2 didn't
> > > match, but it was because it was expected to partially match the
> > > generated output of a previous define_peephole2, which matched and
> > > modified the last insn of a sequence to be matched.  I had assumed
> > > that the algorithm backed-up the size of the match-buffer, thereby
> > > exposing newly created opportunities *with sufficient context* 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 | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > >
> > > diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
> > > index 07bf8bdebffb..300d104d58ab 100644
> > > --- a/gcc/doc/md.texi
> > > +++ b/gcc/doc/md.texi
> > > @@ -9362,6 +9362,15 @@ 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.  Matches are attempted in order of @code{define_peephole2}
> > > +appearance in the @file{md} file.  After a successful replacement,
> > > +scanning for further opportunities for @code{define_peephole2}, resumes
> > > +with the first generated replacement insn as the first insn to be
> > > +matched against all @code{define_peephole2}.  For the example above,
> > > +after its successful replacement, the first insn that can be matched by
> > > +a @code{define_peephole2} is @code{(set (match_dup 4) (match_dup 1))}.
> > > +
> > >  @end ifset
> > >  @ifset INTERNALS
> > >  @node Insn Attributes
> > > --
> > > 2.30.2
> > >
> >

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-05-04  9:45 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-18 17:55 [PATCH] doc: Document order of define_peephole2 scanning Hans-Peter Nilsson
2023-04-18 18:32 ` Paul Koning
2023-04-18 18:44   ` Hans-Peter Nilsson
2023-04-19  3:15     ` [PATCH v2] " Hans-Peter Nilsson
2023-04-19  4:06       ` Hans-Peter Nilsson
2023-04-19 16:59         ` [PATCH v3] " Hans-Peter Nilsson
     [not found]           ` <4E63CF4D-C355-4053-8EA4-5700C1B938E3@gmail.com>
2023-04-19 20:16             ` Bernhard Reutner-Fischer
2023-04-20  2:57               ` Hans-Peter Nilsson
2023-04-26 23:55           ` Ping: " Hans-Peter Nilsson
2023-05-04  0:09             ` 2nd " Hans-Peter Nilsson
2023-05-04  9:45               ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).