public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Segher Boessenkool <segher@kernel.crashing.org>
To: "Kewen.Lin" <linkw@linux.ibm.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>,
	Bill Schmidt <wschmidt@linux.ibm.com>,
	David Edelsohn <dje.gcc@gmail.com>
Subject: Re: [PATCH] rs6000: Parameterize some const values for density test
Date: Fri, 17 Sep 2021 17:26:27 -0500	[thread overview]
Message-ID: <20210917222627.GY1583@gate.crashing.org> (raw)
In-Reply-To: <b0a97dfc-870f-069a-992d-49f2942b36dc@linux.ibm.com>

Hi!

On Wed, Sep 15, 2021 at 04:52:49PM +0800, Kewen.Lin wrote:
> This patch follows the discussion here[1], where Segher suggested
> parameterizing those exact magic constants for density heuristics,
> to make it easier to tweak if need.
> 
> Since these heuristics are quite internal, I make these parameters
> as undocumented and be mainly used by developers.

Okido.

> +      if (data->nloads > (unsigned int) rs6000_density_load_num_threshold
> +	  && load_pct > (unsigned int) rs6000_density_load_pct_threshold)

Those variables are unsigned int already.  Don't cast please.

> +-param=rs6000-density-pct-threshold=
> +Target Undocumented Joined UInteger Var(rs6000_density_pct_threshold) Init(85) IntegerRange(0, 99) Param

So make this and all other percentages (0, 100) please.

> +When costing for loop vectorization, we probably need to penalize the loop body cost if the existing cost model may not adequately reflect delays from unavailable vector resources.  We collect the cost for vectorized statements and non-vectorized statements separately, check the proportion of vec_cost to total cost of vec_cost and non vec_cost, and penalize only if the proportion exceeds the threshold specified by this parameter.  The default value is 85.

It would be good if we can use line breaks in the source code for things
like this, but I don't think we can.  This message is mainly used for
"--help=param", and it is good there to have as short messages as you
can.  But given the nature of params you need quite a few words often,
and you do not want to say so little that things are no clear, either.

So, dunno :-)

Oksy for trunk with these fixes and what Bill mentioned in the other
thread.  Thanks!


Segher

  parent reply	other threads:[~2021-09-17 22:27 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-15  8:52 Kewen.Lin
2021-09-17 16:27 ` Bill Schmidt
2021-09-17 22:15   ` Segher Boessenkool
2021-09-21  3:27   ` Kewen.Lin
2021-09-17 22:26 ` Segher Boessenkool [this message]
2021-09-21  5:47   ` Kewen.Lin
2021-09-21 12:03     ` Segher Boessenkool
2021-09-22  5:17       ` Kewen.Lin

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=20210917222627.GY1583@gate.crashing.org \
    --to=segher@kernel.crashing.org \
    --cc=dje.gcc@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=linkw@linux.ibm.com \
    --cc=wschmidt@linux.ibm.com \
    /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).