Re: [PATCH] doc clarification: DONE and FAIL in define_split and define_peephole2

2018-07-06 Thread Paul Koning



> On Jul 6, 2018, at 12:20 PM, Richard Sandiford  
> wrote:
> 
> Double empty line.
> 
> OK otherwise, thanks.  (Think this counts as a gen* patch.)
> 
> Richard

Thanks.  Committed as shown below.

paul

ChangeLog:

2018-07-06  Paul Koning  

* doc/md.texi (define_split): Document DONE and FAIL.
(define_peephole2): Ditto.

Index: doc/md.texi
===
--- doc/md.texi (revision 262478)
+++ doc/md.texi (working copy)
@@ -8060,6 +8060,30 @@ those in @code{define_expand}, however, these stat
 generate any new pseudo-registers.  Once reload has completed, they also
 must not allocate any space in the stack frame.
 
+There are two special macros defined for use in the preparation statements:
+@code{DONE} and @code{FAIL}.  Use them with a following semicolon,
+as a statement.
+
+@table @code
+
+@findex DONE
+@item DONE
+Use the @code{DONE} macro to end RTL generation for the splitter.  The
+only RTL insns generated as replacement for the matched input insn will
+be those already emitted by explicit calls to @code{emit_insn} within
+the preparation statements; the replacement pattern is not used.
+
+@findex FAIL
+@item FAIL
+Make the @code{define_split} fail on this occasion.  When a @code{define_split}
+fails, it means that the splitter was not truly available for the inputs
+it was given, and the input insn will not be split.
+@end table
+
+If the preparation falls through (invokes neither @code{DONE} nor
+@code{FAIL}), then the @code{define_split} uses the replacement
+template.
+
 Patterns are matched against @var{insn-pattern} in two different
 circumstances.  If an insn needs to be split for delay slot scheduling
 or insn scheduling, the insn is already known to be valid, which means
@@ -8615,6 +8639,33 @@ so here's a silly made-up example:
   "")
 @end smallexample
 
+There are two special macros defined for use in the preparation statements:
+@code{DONE} and @code{FAIL}.  Use them with a following semicolon,
+as a statement.
+
+@table @code
+
+@findex DONE
+@item DONE
+Use the @code{DONE} macro to end RTL generation for the peephole.  The
+only RTL insns generated as replacement for the matched input insn will
+be those already emitted by explicit calls to @code{emit_insn} within
+the preparation statements; the replacement pattern is not used.
+
+@findex FAIL
+@item FAIL
+Make the @code{define_peephole2} fail on this occasion.  When a 
@code{define_peephole2}
+fails, it means that the replacement was not truly available for the
+particular inputs it was given.  In that case, GCC may still apply a
+later @code{define_peephole2} that also matches the given insn pattern.
+(Note that this is different from @code{define_split}, where @code{FAIL}
+prevents the input insn from being split at all.)
+@end table
+
+If the preparation falls through (invokes neither @code{DONE} nor
+@code{FAIL}), then the @code{define_peephole2} uses the replacement
+template.
+
 @noindent
 If we had not added the @code{(match_dup 4)} in the middle of the input
 sequence, it might have been the case that the register we chose at the



Re: [PATCH] doc clarification: DONE and FAIL in define_split and define_peephole2

2018-07-06 Thread Jeff Law
On 07/06/2018 10:20 AM, Richard Sandiford wrote:
> Paul Koning  writes:
>> @@ -8615,6 +8639,34 @@ so here's a silly made-up example:
>>"")
>>  @end smallexample
>>  
>> +There are two special macros defined for use in the preparation statements:
>> +@code{DONE} and @code{FAIL}.  Use them with a following semicolon,
>> +as a statement.
>> +
>> +@table @code
>> +
>> +@findex DONE
>> +@item DONE
>> +Use the @code{DONE} macro to end RTL generation for the peephole.  The
>> +only RTL insns generated as replacement for the matched input insn will
>> +be those already emitted by explicit calls to @code{emit_insn} within
>> +the preparation statements; the replacement pattern is not used.
>> +
>> +@findex FAIL
>> +@item FAIL
>> +Make the @code{define_peephole2} fail on this occasion.  When a 
>> @code{define_peephole2}
>> +fails, it means that the replacement was not truly available for the
>> +particular inputs it was given.  In that case, GCC may still apply a
>> +later @code{define_peephole2} that also matches the given insn pattern.
>> +(Note that this is different from @code{define_split}, where @code{FAIL}
>> +prevents the input insn from being split at all.)
>> +@end table
>> +
>> +If the preparation falls through (invokes neither @code{DONE} nor
>> +@code{FAIL}), then the @code{define_peephole2} uses the replacement
>> +template.
>> +
>> +
>>  @noindent
>>  If we had not added the @code{(match_dup 4)} in the middle of the input
>>  sequence, it might have been the case that the register we chose at the
> 
> Double empty line.
> 
> OK otherwise, thanks.  (Think this counts as a gen* patch.)
:-)  Close enough for me.

Jeff


Re: [PATCH] doc clarification: DONE and FAIL in define_split and define_peephole2

2018-07-06 Thread Richard Sandiford
Paul Koning  writes:
> @@ -8615,6 +8639,34 @@ so here's a silly made-up example:
>"")
>  @end smallexample
>  
> +There are two special macros defined for use in the preparation statements:
> +@code{DONE} and @code{FAIL}.  Use them with a following semicolon,
> +as a statement.
> +
> +@table @code
> +
> +@findex DONE
> +@item DONE
> +Use the @code{DONE} macro to end RTL generation for the peephole.  The
> +only RTL insns generated as replacement for the matched input insn will
> +be those already emitted by explicit calls to @code{emit_insn} within
> +the preparation statements; the replacement pattern is not used.
> +
> +@findex FAIL
> +@item FAIL
> +Make the @code{define_peephole2} fail on this occasion.  When a 
> @code{define_peephole2}
> +fails, it means that the replacement was not truly available for the
> +particular inputs it was given.  In that case, GCC may still apply a
> +later @code{define_peephole2} that also matches the given insn pattern.
> +(Note that this is different from @code{define_split}, where @code{FAIL}
> +prevents the input insn from being split at all.)
> +@end table
> +
> +If the preparation falls through (invokes neither @code{DONE} nor
> +@code{FAIL}), then the @code{define_peephole2} uses the replacement
> +template.
> +
> +
>  @noindent
>  If we had not added the @code{(match_dup 4)} in the middle of the input
>  sequence, it might have been the case that the register we chose at the

Double empty line.

OK otherwise, thanks.  (Think this counts as a gen* patch.)

Richard


Re: [PATCH] doc clarification: DONE and FAIL in define_split and define_peephole2

2018-07-06 Thread Paul Koning
Thanks Richard.  Some comments, and an updated proposed patch.

paul


> On Jul 6, 2018, at 9:04 AM, Richard Sandiford  
> wrote:
> 
> ...
>> @@ -8232,6 +8256,15 @@ functionality as two separate @code{define_insn} a
>> patterns.  It exists for compactness, and as a maintenance tool to prevent
>> having to ensure the two patterns' templates match.
>> 
>> +In @code{define_insn_and_split}, the output template is usually simply
>> +@samp{#} since the assembly output is done by @code{define_insn}
>> +statements matching the generated insns, not by this
>> +@code{define_insn_and_split} statement.  But if @code{FAIL} is used in
>> +the preparation statements for certain input insns, those will not be
>> +split and during assembly output will again match this
>> +@code{define_insn_and_split}.  In that case, the appropriate assembly
>> +output statements are needed in the output template.
>> +
> 
> I agree "#" on its own is relatively common, but it's also not that
> unusual to have a define_insn_and_split in which the define_insn part
> handles simple alternatives directly and leaves more complex ones to
> be split.  Maybe that's more common on RISC-like targets.
> 
> Also, the define_split matches the template independently of the
> define_insn, so it can sometimes split insns that match an earlier
> define_insn rather than the one in the define_insn_and_split.
> (That might be bad practice.)  So using "#" and FAIL together is valid
> if the FAIL only happens for cases that match earlier define_insns.
> 
> Another case is when the define_split condition doesn't start with
> "&&" and is less strict than the define_insn condition.  This can
> be useful if the define_split is supposed to match patterns created
> by combine.
> 
> So maybe we should instead expand the FAIL documentation to say that
> a define_split must not FAIL when splitting an instruction whose
> output template is "#".

I played with this a bit and couldn't come up with a good wording.  The case is 
already covered; I was trying to make it clearer but your comments indicate the 
picture is bigger than I thought.

>> @@ -8615,6 +8648,31 @@ so here's a silly made-up example:
>>   "")
>> @end smallexample
>> 
>> +There are two special macros defined for use in the preparation statements:
>> +@code{DONE} and @code{FAIL}.  Use them with a following semicolon,
>> +as a statement.
>> +
>> +@table @code
>> +
>> +@findex DONE
>> +@item DONE
>> +Use the @code{DONE} macro to end RTL generation for the peephole.  The
>> +only RTL insns generated as replacement for the matched input insn will
>> +be those already emitted by explicit calls to @code{emit_insn} within
>> +the preparation statements; the replacement pattern is not used.
>> +
>> +@findex FAIL
>> +@item FAIL
>> +Make the @code{define_peephole2} fail on this occasion.  When a 
>> @code{define_peephole2}
>> +fails, it means that the replacement was not truly available for the
>> +particular inputs it was given, and the input insns are left unchanged.
> 
> If it FAILs, GCC will try to apply later define_peehole2s instead.
> (This is in contrast to define_split, so it's a bit inconsistent.
> Would be easy to make define_split behave the same way if there was a
> motivating case.)

Interesting.  I added words to the FAIL description of both define_split and 
define_peephole2 to state the behavior and highlight the difference.

ChangeLog:

2018-07-05  Paul Koning  

* doc/md.texi (define_split): Document DONE and FAIL.  Describe
interaction with usual "#" output template in
define_insn_and_split.
(define_peephole2): Document DONE and FAIL.

Index: doc/md.texi
===
--- doc/md.texi (revision 262455)
+++ doc/md.texi (working copy)
@@ -8060,6 +8060,30 @@ those in @code{define_expand}, however, these stat
 generate any new pseudo-registers.  Once reload has completed, they also
 must not allocate any space in the stack frame.
 
+There are two special macros defined for use in the preparation statements:
+@code{DONE} and @code{FAIL}.  Use them with a following semicolon,
+as a statement.
+
+@table @code
+
+@findex DONE
+@item DONE
+Use the @code{DONE} macro to end RTL generation for the splitter.  The
+only RTL insns generated as replacement for the matched input insn will
+be those already emitted by explicit calls to @code{emit_insn} within
+the preparation statements; the replacement pattern is not used.
+
+@findex FAIL
+@item FAIL
+Make the @code{define_split} fail on this occasion.  When a @code{define_split}
+fails, it means that the splitter was not truly available for the inputs
+it was given, and the input insn will not be split.
+@end table
+
+If the preparation falls through (invokes neither @code{DONE} nor
+@code{FAIL}), then the @code{define_split} uses the replacement
+template.
+
 Patterns are matched against @var{insn-pattern} in two different
 circumstances.  If an insn needs to be split for d

Re: [PATCH] doc clarification: DONE and FAIL in define_split and define_peephole2

2018-07-06 Thread Richard Sandiford
Paul Koning  writes:
> Currently DONE and FAIL are documented only for define_expand, but
> they also work in essentially the same way for define_split and
> define_peephole2.
>
> If FAIL is used in a define_insn_and_split, the output pattern cannot
> be the usual "#" dummy value.
>
> This patch updates the doc to describe those cases.  Ok for trunk?
>
>   paul
>
> ChangeLog:
>
> 2018-07-05  Paul Koning  
>
>   * doc/md.texi (define_split): Document DONE and FAIL.  Describe
>   interaction with usual "#" output template in
>   define_insn_and_split.
>   (define_peephole2): Document DONE and FAIL.
>
> Index: doc/md.texi
> ===
> --- doc/md.texi   (revision 262455)
> +++ doc/md.texi   (working copy)
> @@ -8060,6 +8060,30 @@ those in @code{define_expand}, however, these stat
>  generate any new pseudo-registers.  Once reload has completed, they also
>  must not allocate any space in the stack frame.
>  
> +There are two special macros defined for use in the preparation statements:
> +@code{DONE} and @code{FAIL}.  Use them with a following semicolon,
> +as a statement.
> +
> +@table @code
> +
> +@findex DONE
> +@item DONE
> +Use the @code{DONE} macro to end RTL generation for the splitter.  The
> +only RTL insns generated as replacement for the matched input insn will
> +be those already emitted by explicit calls to @code{emit_insn} within
> +the preparation statements; the replacement pattern is not used.
> +
> +@findex FAIL
> +@item FAIL
> +Make the @code{define_split} fail on this occasion.  When a 
> @code{define_split}
> +fails, it means that the splitter was not truly available for the inputs
> +it was given, and this split is not done.
> +@end table
> +
> +If the preparation falls through (invokes neither @code{DONE} nor
> +@code{FAIL}), then the @code{define_split} uses the replacement
> +template.
> +
>  Patterns are matched against @var{insn-pattern} in two different
>  circumstances.  If an insn needs to be split for delay slot scheduling
>  or insn scheduling, the insn is already known to be valid, which means

Looks good.

> @@ -8232,6 +8256,15 @@ functionality as two separate @code{define_insn} a
>  patterns.  It exists for compactness, and as a maintenance tool to prevent
>  having to ensure the two patterns' templates match.
>  
> +In @code{define_insn_and_split}, the output template is usually simply
> +@samp{#} since the assembly output is done by @code{define_insn}
> +statements matching the generated insns, not by this
> +@code{define_insn_and_split} statement.  But if @code{FAIL} is used in
> +the preparation statements for certain input insns, those will not be
> +split and during assembly output will again match this
> +@code{define_insn_and_split}.  In that case, the appropriate assembly
> +output statements are needed in the output template.
> +

I agree "#" on its own is relatively common, but it's also not that
unusual to have a define_insn_and_split in which the define_insn part
handles simple alternatives directly and leaves more complex ones to
be split.  Maybe that's more common on RISC-like targets.

Also, the define_split matches the template independently of the
define_insn, so it can sometimes split insns that match an earlier
define_insn rather than the one in the define_insn_and_split.
(That might be bad practice.)  So using "#" and FAIL together is valid
if the FAIL only happens for cases that match earlier define_insns.

Another case is when the define_split condition doesn't start with
"&&" and is less strict than the define_insn condition.  This can
be useful if the define_split is supposed to match patterns created
by combine.

So maybe we should instead expand the FAIL documentation to say that
a define_split must not FAIL when splitting an instruction whose
output template is "#".

> @@ -8615,6 +8648,31 @@ so here's a silly made-up example:
>"")
>  @end smallexample
>  
> +There are two special macros defined for use in the preparation statements:
> +@code{DONE} and @code{FAIL}.  Use them with a following semicolon,
> +as a statement.
> +
> +@table @code
> +
> +@findex DONE
> +@item DONE
> +Use the @code{DONE} macro to end RTL generation for the peephole.  The
> +only RTL insns generated as replacement for the matched input insn will
> +be those already emitted by explicit calls to @code{emit_insn} within
> +the preparation statements; the replacement pattern is not used.
> +
> +@findex FAIL
> +@item FAIL
> +Make the @code{define_peephole2} fail on this occasion.  When a 
> @code{define_peephole2}
> +fails, it means that the replacement was not truly available for the
> +particular inputs it was given, and the input insns are left unchanged.

If it FAILs, GCC will try to apply later define_peehole2s instead.
(This is in contrast to define_split, so it's a bit inconsistent.
Would be easy to make define_split behave the same way if there was a
motivating case.)

Thanks,
Richa

[PATCH] doc clarification: DONE and FAIL in define_split and define_peephole2

2018-07-05 Thread Paul Koning
Currently DONE and FAIL are documented only for define_expand, but they also 
work in essentially the same way for define_split and define_peephole2.

If FAIL is used in a define_insn_and_split, the output pattern cannot be the 
usual "#" dummy value. 

This patch updates the doc to describe those cases.  Ok for trunk?

paul

ChangeLog:

2018-07-05  Paul Koning  

* doc/md.texi (define_split): Document DONE and FAIL.  Describe
interaction with usual "#" output template in
define_insn_and_split.
(define_peephole2): Document DONE and FAIL.

Index: doc/md.texi
===
--- doc/md.texi (revision 262455)
+++ doc/md.texi (working copy)
@@ -8060,6 +8060,30 @@ those in @code{define_expand}, however, these stat
 generate any new pseudo-registers.  Once reload has completed, they also
 must not allocate any space in the stack frame.
 
+There are two special macros defined for use in the preparation statements:
+@code{DONE} and @code{FAIL}.  Use them with a following semicolon,
+as a statement.
+
+@table @code
+
+@findex DONE
+@item DONE
+Use the @code{DONE} macro to end RTL generation for the splitter.  The
+only RTL insns generated as replacement for the matched input insn will
+be those already emitted by explicit calls to @code{emit_insn} within
+the preparation statements; the replacement pattern is not used.
+
+@findex FAIL
+@item FAIL
+Make the @code{define_split} fail on this occasion.  When a @code{define_split}
+fails, it means that the splitter was not truly available for the inputs
+it was given, and this split is not done.
+@end table
+
+If the preparation falls through (invokes neither @code{DONE} nor
+@code{FAIL}), then the @code{define_split} uses the replacement
+template.
+
 Patterns are matched against @var{insn-pattern} in two different
 circumstances.  If an insn needs to be split for delay slot scheduling
 or insn scheduling, the insn is already known to be valid, which means
@@ -8232,6 +8256,15 @@ functionality as two separate @code{define_insn} a
 patterns.  It exists for compactness, and as a maintenance tool to prevent
 having to ensure the two patterns' templates match.
 
+In @code{define_insn_and_split}, the output template is usually simply
+@samp{#} since the assembly output is done by @code{define_insn}
+statements matching the generated insns, not by this
+@code{define_insn_and_split} statement.  But if @code{FAIL} is used in
+the preparation statements for certain input insns, those will not be
+split and during assembly output will again match this
+@code{define_insn_and_split}.  In that case, the appropriate assembly
+output statements are needed in the output template.
+
 @end ifset
 @ifset INTERNALS
 @node Including Patterns
@@ -8615,6 +8648,31 @@ so here's a silly made-up example:
   "")
 @end smallexample
 
+There are two special macros defined for use in the preparation statements:
+@code{DONE} and @code{FAIL}.  Use them with a following semicolon,
+as a statement.
+
+@table @code
+
+@findex DONE
+@item DONE
+Use the @code{DONE} macro to end RTL generation for the peephole.  The
+only RTL insns generated as replacement for the matched input insn will
+be those already emitted by explicit calls to @code{emit_insn} within
+the preparation statements; the replacement pattern is not used.
+
+@findex FAIL
+@item FAIL
+Make the @code{define_peephole2} fail on this occasion.  When a 
@code{define_peephole2}
+fails, it means that the replacement was not truly available for the
+particular inputs it was given, and the input insns are left unchanged.
+@end table
+
+If the preparation falls through (invokes neither @code{DONE} nor
+@code{FAIL}), then the @code{define_peephole2} uses the replacement
+template.
+
+
 @noindent
 If we had not added the @code{(match_dup 4)} in the middle of the input
 sequence, it might have been the case that the register we chose at the