Re: [PATCH v2] doc: md.texi (Insn Splitting): Tweak wording for readability.
On 3/21/23 08:22, Hans-Peter Nilsson via Gcc-patches wrote: From: Hans-Peter Nilsson CC: , 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
Ping #2: [PATCH v2] doc: md.texi (Insn Splitting): Tweak wording for readability.
> From: Hans-Peter Nilsson
> 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
> ---
> 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.
> From: Hans-Peter Nilsson
> CC: ,
> 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
> ---
> 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.
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
[PATCH v2] doc: md.texi (Insn Splitting): Tweak wording for readability.
> Date: Mon, 13 Mar 2023 22:31:21 -0600
> From: 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.
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
---
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
