public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Gopalasubramanian, Ganesh" <Ganesh.Gopalasubramanian@amd.com>
To: Richard Biener <rguenther@suse.de>
Cc: Uros Bizjak <ubizjak@gmail.com>,
	"gcc-patches@gcc.gnu.org"	<gcc-patches@gcc.gnu.org>
Subject: RE: [PATCH] Fix bdverN vector cost of cond_[not_]taken_branch_cost
Date: Mon, 13 Apr 2015 05:49:00 -0000	[thread overview]
Message-ID: <EB4625145972F94C9680D8CADD65161578FE595E@SATLEXDAG02.amd.com> (raw)
In-Reply-To: <alpine.LSU.2.11.1504080935320.5207@zhemvz.fhfr.qr>

>Note that before the fixes for PR64909 the epilogue/prologue loops had very large costs associated due to a bug in the cost model implementation.  After the fix their cost is reasonable but the cost of the extra jumps is way under-accounted for due to the numbers for >cond_taken_branch_cost and cond_not_taken_branch_cost.
> The proposes match mitigates that somewhat.

Richard! The patch is good. We are done with our benchmarking and found no regressions.

> How did you arrive at the original cost model?

The original cost model as you suspect is not based on architecture alone. Those are the numbers arrived at by analyzing benchmarks and the cost model bugs then. These initial numbers were copied for subsequent architectures too.
Cost assignments saying "scalar_stmt_cost = 6" and "scalar load_cost = 4" doesn't make sense at all. We will have a look into it.

Regards
Ganesh

-----Original Message-----
From: Richard Biener [mailto:rguenther@suse.de] 
Sent: Wednesday, April 08, 2015 1:08 PM
To: Gopalasubramanian, Ganesh
Cc: Uros Bizjak; gcc-patches@gcc.gnu.org
Subject: RE: [PATCH] Fix bdverN vector cost of cond_[not_]taken_branch_cost

On Wed, 8 Apr 2015, Gopalasubramanian, Ganesh wrote:

> > I have added a person from AMD to comment on the decision.
> > Otherwise, the patch looks OK, but please wait a couple of days for possible comments.
> 
> Thank you Uros!
> I am checking the changes with few tests and benchmarking them.
> Please wait for a couple of days.

Note that before the fixes for PR64909 the epilogue/prologue loops had very large costs associated due to a bug in the cost model implementation.  After the fix their cost is reasonable but the cost of the extra jumps is way under-accounted for due to the numbers for cond_taken_branch_cost and cond_not_taken_branch_cost.
The proposes match mitigates that somewhat.

How did you arrive at the original cost model?

Thanks,
Richard.

--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu, Graham Norton HRB 21284 (AG Nuernberg)

      reply	other threads:[~2015-04-13  5:49 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-07  8:19 Richard Biener
2015-04-07 11:20 ` Uros Bizjak
2015-04-08  6:39   ` Gopalasubramanian, Ganesh
2015-04-08  7:38     ` Richard Biener
2015-04-13  5:49       ` Gopalasubramanian, Ganesh [this message]

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=EB4625145972F94C9680D8CADD65161578FE595E@SATLEXDAG02.amd.com \
    --to=ganesh.gopalasubramanian@amd.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=rguenther@suse.de \
    --cc=ubizjak@gmail.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).