public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Sandra Loosemore <sandra@codesourcery.com>
To: Hans-Peter Nilsson <hp@axis.com>, <gcc-patches@gcc.gnu.org>,
	<hubicka@ucw.cz>
Subject: Re: [PATCH] doc: md.texi (Insn Splitting): Tweak wording for readability.
Date: Mon, 13 Mar 2023 22:31:21 -0600	[thread overview]
Message-ID: <ca30d2ae-28c1-a310-ec51-1bf18a4e610c@codesourcery.com> (raw)
In-Reply-To: <20230314012536.2789120417@pchp3.se.axis.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.

> ---
>   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

  reply	other threads:[~2023-03-14  4:31 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-14  1:25 Hans-Peter Nilsson
2023-03-14  4:31 ` Sandra Loosemore [this message]
2023-03-14 16:04   ` [PATCH v2] " Hans-Peter Nilsson
2023-03-14 16:43     ` Sandra Loosemore
2023-03-21 14:22     ` Hans-Peter Nilsson
2023-04-03  5:25       ` Jeff Law
2023-03-28 13:29     ` Ping #2: " Hans-Peter Nilsson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ca30d2ae-28c1-a310-ec51-1bf18a4e610c@codesourcery.com \
    --to=sandra@codesourcery.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hp@axis.com \
    --cc=hubicka@ucw.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).