[PATCH] doc: md.texi (Insn Splitting): Tweak wording for readability.

[PATCH] doc: md.texi (Insn Splitting): Tweak wording for readability.

[Hans-Peter Nilsson]

Jan, did I get this right?  This was from your
r0-36413-g6b24c25948265c / svn r44249, now on its 22nd year!

I spot-checked the pdf for readability.  Also calling on a
doc maintainer to check grammos etc.  Ok to commit?

-- >8 --
I needed to check what was allowed in a define_split, but
had problems understanding what was meant by "Splitting of
jump instruction into sequence that over by another jump
instruction".

	* doc/md.texi (Insn Splitting): Tweak wording for readability.
---
 gcc/doc/md.texi | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index 8e3113599fdc..786365143179 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -8756,21 +8756,21 @@ insns that don't.  Instead, write two separate @code{define_split}
 definitions, one for the insns that are valid and one for the insns that
 are not valid.
 
-The splitter is allowed to split jump instructions into sequence of
-jumps or create new jumps in while splitting non-jump instructions.  As
-the control flow graph and branch prediction information needs to be updated,
-several restriction apply.
-
-Splitting of jump instruction into sequence that over by another jump
-instruction is always valid, as compiler expect identical behavior of new
-jump.  When new sequence contains multiple jump instructions or new labels,
-more assistance is needed.  Splitter is required to create only unconditional
-jumps, or simple conditional jump instructions.  Additionally it must attach a
-@code{REG_BR_PROB} note to each conditional jump.  A global variable
-@code{split_branch_probability} holds the probability of the original branch in case
-it was a simple conditional jump, @minus{}1 otherwise.  To simplify
-recomputing of edge frequencies, the new sequence is required to have only
-forward jumps to the newly created labels.
+The splitter is allowed to split jump instructions into a sequence of jumps or
+create new jumps while splitting non-jump instructions.  As the control flow
+graph and branch prediction information needs to be updated, several
+restrictions apply.
+
+Splitting of a jump instruction into a sequence that has another jump
+instruction is always valid, as the compiler expects identical behavior of the
+new jump.  When the new sequence contains multiple jump instructions or new
+labels, more assistance is needed.  The splitter is required to create only
+unconditional jumps, or simple conditional jump instructions.  Additionally it
+must attach a @code{REG_BR_PROB} note to each conditional jump.  A global
+variable @code{split_branch_probability} holds the probability of the original
+branch in case it was a simple conditional jump, @minus{}1 otherwise.  To
+simplify recomputing of edge frequencies, the new sequence is required to have
+only forward jumps to the newly created labels.
 
 @findex define_insn_and_split
 For the common case where the pattern of a define_split exactly matches the
-- 
2.30.2

Re: [PATCH] doc: md.texi (Insn Splitting): Tweak wording for readability.

[Sandra Loosemore]

On 3/13/23 19:25, Hans-Peter Nilsson via Gcc-patches wrote:
> Jan, did I get this right?  This was from your
> r0-36413-g6b24c25948265c / svn r44249, now on its 22nd year!
> 
> I spot-checked the pdf for readability.  Also calling on a
> doc maintainer to check grammos etc.  Ok to commit?
> 
> -- >8 --
> I needed to check what was allowed in a define_split, but
> had problems understanding what was meant by "Splitting of
> jump instruction into sequence that over by another jump
> instruction".
> 
> 	* doc/md.texi (Insn Splitting): Tweak wording for readability.

Thanks for noticing this!  I can't comment on technical correctness, but 
I do have some further suggestions on wording below.

> ---
>   gcc/doc/md.texi | 30 +++++++++++++++---------------
>   1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
> index 8e3113599fdc..786365143179 100644
> --- a/gcc/doc/md.texi
> +++ b/gcc/doc/md.texi
> @@ -8756,21 +8756,21 @@ insns that don't.  Instead, write two separate @code{define_split}
>   definitions, one for the insns that are valid and one for the insns that
>   are not valid.
>   
> -The splitter is allowed to split jump instructions into sequence of
> -jumps or create new jumps in while splitting non-jump instructions.  As
> -the control flow graph and branch prediction information needs to be updated,
> -several restriction apply.
> -
> -Splitting of jump instruction into sequence that over by another jump
> -instruction is always valid, as compiler expect identical behavior of new
> -jump.  When new sequence contains multiple jump instructions or new labels,
> -more assistance is needed.  Splitter is required to create only unconditional
> -jumps, or simple conditional jump instructions.  Additionally it must attach a
> -@code{REG_BR_PROB} note to each conditional jump.  A global variable
> -@code{split_branch_probability} holds the probability of the original branch in case
> -it was a simple conditional jump, @minus{}1 otherwise.  To simplify
> -recomputing of edge frequencies, the new sequence is required to have only
> -forward jumps to the newly created labels.
> +The splitter is allowed to split jump instructions into a sequence of jumps or
> +create new jumps while splitting non-jump instructions.  As the control flow
> +graph and branch prediction information needs to be updated, several
> +restrictions apply.

Maybe "needs to be updated after the splitter runs"?

> +
> +Splitting of a jump instruction into a sequence that has another jump
> +instruction is always valid, as the compiler expects identical behavior of the
> +new jump.  

Maybe "another jump instruction to the same label"?

> When the new sequence contains multiple jump instructions or new
> +labels, more assistance is needed.  The splitter is required to create only

s/required/permitted/

> +unconditional jumps, or simple conditional jump instructions.  Additionally it
> +must attach a @code{REG_BR_PROB} note to each conditional jump.  A global
> +variable @code{split_branch_probability} holds the probability of the original
> +branch in case it was a simple conditional jump, @minus{}1 otherwise.  To
> +simplify recomputing of edge frequencies, the new sequence is required to have
> +only forward jumps to the newly created labels.

s/required/permitted/ again
s/newly created/newly-created/

>   
>   @findex define_insn_and_split
>   For the common case where the pattern of a define_split exactly matches the

-Sandra

[PATCH v2] doc: md.texi (Insn Splitting): Tweak wording for readability.

[Hans-Peter Nilsson]

> Date: Mon, 13 Mar 2023 22:31:21 -0600
> From: Sandra Loosemore <sandra@codesourcery.com>

> On 3/13/23 19:25, Hans-Peter Nilsson via Gcc-patches wrote:
> > Jan, did I get this right?  This was from your
> > r0-36413-g6b24c25948265c / svn r44249, now on its 22nd year!
> > 
> > I spot-checked the pdf for readability.  Also calling on a
> > doc maintainer to check grammos etc.  Ok to commit?
> > 
> > -- >8 --
> > I needed to check what was allowed in a define_split, but
> > had problems understanding what was meant by "Splitting of
> > jump instruction into sequence that over by another jump
> > instruction".
> > 
> > 	* doc/md.texi (Insn Splitting): Tweak wording for readability.
> 
> Thanks for noticing this!  I can't comment on technical correctness, but 
> I do have some further suggestions on wording below.

Thank you for the review!  Updated version below with your
suggestions.  When spot-checking the pdf I noticed a strange
split of the page after the next after the section I
changed: last on page 484 "17.17 Including Patterns in
Machine Descriptions", there's a "(include" last on the page
and "pathname)" on top of page 485.  I'm afraid this patch
triggered that.  IMHO it'd be wrong to diddle with
formatting of *that* in *this* patch, instead leaving it to
a follow-up-patch.  I think the obvious fix is to *not*
split up (include pathname)" because that just looks odd
even without the page end in-between.  Right?

-- >8 --
I needed to check what was allowed in a define_split, but
had problems understanding what was meant by "Splitting of
jump instruction into sequence that over by another jump
instruction".

	* doc/md.texi (Insn Splitting): Tweak wording for readability.

Co-Authored-By: Sandra Loosemore <sandra@codesourcery.com>
---
 gcc/doc/md.texi | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index 8e3113599fdc..134b227b9a93 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -8756,21 +8756,21 @@ insns that don't.  Instead, write two separate @code{define_split}
 definitions, one for the insns that are valid and one for the insns that
 are not valid.
 
-The splitter is allowed to split jump instructions into sequence of
-jumps or create new jumps in while splitting non-jump instructions.  As
-the control flow graph and branch prediction information needs to be updated,
-several restriction apply.
-
-Splitting of jump instruction into sequence that over by another jump
-instruction is always valid, as compiler expect identical behavior of new
-jump.  When new sequence contains multiple jump instructions or new labels,
-more assistance is needed.  Splitter is required to create only unconditional
-jumps, or simple conditional jump instructions.  Additionally it must attach a
-@code{REG_BR_PROB} note to each conditional jump.  A global variable
-@code{split_branch_probability} holds the probability of the original branch in case
-it was a simple conditional jump, @minus{}1 otherwise.  To simplify
-recomputing of edge frequencies, the new sequence is required to have only
-forward jumps to the newly created labels.
+The splitter is allowed to split jump instructions into a sequence of jumps or
+create new jumps while splitting non-jump instructions.  As the control flow
+graph and branch prediction information needs to be updated after the splitter
+runs, several restrictions apply.
+
+Splitting of a jump instruction into a sequence that has another jump
+instruction to the same label is always valid, as the compiler expects
+identical behavior of the new jump.  When the new sequence contains multiple
+jump instructions or new labels, more assistance is needed.  The splitter is
+permitted to create only unconditional jumps, or simple conditional jump
+instructions.  Additionally it must attach a @code{REG_BR_PROB} note to each
+conditional jump.  A global variable @code{split_branch_probability} holds the
+probability of the original branch in case it was a simple conditional jump,
+@minus{}1 otherwise.  To simplify recomputing of edge frequencies, the new
+sequence is permitted to have only forward jumps to the newly-created labels.
 
 @findex define_insn_and_split
 For the common case where the pattern of a define_split exactly matches the
-- 
2.30.2

brgds, H-P

Re: [PATCH v2] doc: md.texi (Insn Splitting): Tweak wording for readability.

[Sandra Loosemore]

On 3/14/23 10:04, Hans-Peter Nilsson via Gcc-patches wrote:

> Thank you for the review!  Updated version below with your
> suggestions.

This looks fine to me, from a writing perspective at least.

> When spot-checking the pdf I noticed a strange
> split of the page after the next after the section I
> changed: last on page 484 "17.17 Including Patterns in
> Machine Descriptions", there's a "(include" last on the page
> and "pathname)" on top of page 485.  I'm afraid this patch
> triggered that.  IMHO it'd be wrong to diddle with
> formatting of *that* in *this* patch, instead leaving it to
> a follow-up-patch.  I think the obvious fix is to *not*
> split up (include pathname)" because that just looks odd
> even without the page end in-between.  Right?

Yes.  I was skimming through the PDF of the GCC user manual myself last 
week, and noticed a *lot* of problems with both extraneous line breaks 
and lines that are too long and get cut off on the right in preformatted 
text pieces (including the option tables).  I'm sure the internals 
manual is even worse because it's gotten much less clean-up effort than 
the user-facing documentation.

FWIW, I consider fixes to whitespace, adding missing markup like @code, 
etc to be "obvious" changes in the same category as fixing typos. 
There's no need to ask for review of such changes, just go ahead and 
push them and post the patch of what you did.

-Sandra

Re: [PATCH v2] doc: md.texi (Insn Splitting): Tweak wording for readability.

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson <hp@axis.com>
> CC: <gcc-patches@gcc.gnu.org>, <hubicka@ucw.cz>
> Date: Tue, 14 Mar 2023 17:04:43 +0100

Ping on contents (formatting is approved):

> I needed to check what was allowed in a define_split, but
> had problems understanding what was meant by "Splitting of
> jump instruction into sequence that over by another jump
> instruction".
> 
> 	* doc/md.texi (Insn Splitting): Tweak wording for readability.
> 
> Co-Authored-By: Sandra Loosemore <sandra@codesourcery.com>
> ---
>  gcc/doc/md.texi | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
> index 8e3113599fdc..134b227b9a93 100644
> --- a/gcc/doc/md.texi
> +++ b/gcc/doc/md.texi
> @@ -8756,21 +8756,21 @@ insns that don't.  Instead, write two separate @code{define_split}
>  definitions, one for the insns that are valid and one for the insns that
>  are not valid.
>  
> -The splitter is allowed to split jump instructions into sequence of
> -jumps or create new jumps in while splitting non-jump instructions.  As
> -the control flow graph and branch prediction information needs to be updated,
> -several restriction apply.
> -
> -Splitting of jump instruction into sequence that over by another jump
> -instruction is always valid, as compiler expect identical behavior of new
> -jump.  When new sequence contains multiple jump instructions or new labels,
> -more assistance is needed.  Splitter is required to create only unconditional
> -jumps, or simple conditional jump instructions.  Additionally it must attach a
> -@code{REG_BR_PROB} note to each conditional jump.  A global variable
> -@code{split_branch_probability} holds the probability of the original branch in case
> -it was a simple conditional jump, @minus{}1 otherwise.  To simplify
> -recomputing of edge frequencies, the new sequence is required to have only
> -forward jumps to the newly created labels.
> +The splitter is allowed to split jump instructions into a sequence of jumps or
> +create new jumps while splitting non-jump instructions.  As the control flow
> +graph and branch prediction information needs to be updated after the splitter
> +runs, several restrictions apply.
> +
> +Splitting of a jump instruction into a sequence that has another jump
> +instruction to the same label is always valid, as the compiler expects
> +identical behavior of the new jump.  When the new sequence contains multiple
> +jump instructions or new labels, more assistance is needed.  The splitter is
> +permitted to create only unconditional jumps, or simple conditional jump
> +instructions.  Additionally it must attach a @code{REG_BR_PROB} note to each
> +conditional jump.  A global variable @code{split_branch_probability} holds the
> +probability of the original branch in case it was a simple conditional jump,
> +@minus{}1 otherwise.  To simplify recomputing of edge frequencies, the new
> +sequence is permitted to have only forward jumps to the newly-created labels.
>  
>  @findex define_insn_and_split
>  For the common case where the pattern of a define_split exactly matches the
> -- 
> 2.30.2
> 
> brgds, H-P
> 

Ping #2: [PATCH v2] doc: md.texi (Insn Splitting): Tweak wording for readability.

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Tue, 14 Mar 2023 17:04:43 +0100

Ping #2 on contents (formatting is approved):

> -- >8 --
> I needed to check what was allowed in a define_split, but
> had problems understanding what was meant by "Splitting of
> jump instruction into sequence that over by another jump
> instruction".
> 
> 	* doc/md.texi (Insn Splitting): Tweak wording for readability.
> 
> Co-Authored-By: Sandra Loosemore <sandra@codesourcery.com>
> ---
>  gcc/doc/md.texi | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
> index 8e3113599fdc..134b227b9a93 100644
> --- a/gcc/doc/md.texi
> +++ b/gcc/doc/md.texi
> @@ -8756,21 +8756,21 @@ insns that don't.  Instead, write two separate @code{define_split}
>  definitions, one for the insns that are valid and one for the insns that
>  are not valid.
>  
> -The splitter is allowed to split jump instructions into sequence of
> -jumps or create new jumps in while splitting non-jump instructions.  As
> -the control flow graph and branch prediction information needs to be updated,
> -several restriction apply.
> -
> -Splitting of jump instruction into sequence that over by another jump
> -instruction is always valid, as compiler expect identical behavior of new
> -jump.  When new sequence contains multiple jump instructions or new labels,
> -more assistance is needed.  Splitter is required to create only unconditional
> -jumps, or simple conditional jump instructions.  Additionally it must attach a
> -@code{REG_BR_PROB} note to each conditional jump.  A global variable
> -@code{split_branch_probability} holds the probability of the original branch in case
> -it was a simple conditional jump, @minus{}1 otherwise.  To simplify
> -recomputing of edge frequencies, the new sequence is required to have only
> -forward jumps to the newly created labels.
> +The splitter is allowed to split jump instructions into a sequence of jumps or
> +create new jumps while splitting non-jump instructions.  As the control flow
> +graph and branch prediction information needs to be updated after the splitter
> +runs, several restrictions apply.
> +
> +Splitting of a jump instruction into a sequence that has another jump
> +instruction to the same label is always valid, as the compiler expects
> +identical behavior of the new jump.  When the new sequence contains multiple
> +jump instructions or new labels, more assistance is needed.  The splitter is
> +permitted to create only unconditional jumps, or simple conditional jump
> +instructions.  Additionally it must attach a @code{REG_BR_PROB} note to each
> +conditional jump.  A global variable @code{split_branch_probability} holds the
> +probability of the original branch in case it was a simple conditional jump,
> +@minus{}1 otherwise.  To simplify recomputing of edge frequencies, the new
> +sequence is permitted to have only forward jumps to the newly-created labels.
>  
>  @findex define_insn_and_split
>  For the common case where the pattern of a define_split exactly matches the
> -- 
> 2.30.2
> 
> brgds, H-P
> 

Re: [PATCH v2] doc: md.texi (Insn Splitting): Tweak wording for readability.

[Jeff Law]

On 3/21/23 08:22, Hans-Peter Nilsson via Gcc-patches wrote:
>> From: Hans-Peter Nilsson <hp@axis.com>
>> CC: <gcc-patches@gcc.gnu.org>, <hubicka@ucw.cz>
>> Date: Tue, 14 Mar 2023 17:04:43 +0100
> 
> Ping on contents (formatting is approved):
> 
>> I needed to check what was allowed in a define_split, but
>> had problems understanding what was meant by "Splitting of
>> jump instruction into sequence that over by another jump
>> instruction".
>>
OK on content.

jeff