From: James Greenhalgh <james.greenhalgh@arm.com>
To: <gcc-patches@gcc.gnu.org>
Cc: <nd@arm.com>, <richard.guenther@gmail.com>,
<Ramana.Radhakrishnan@arm.com>, <bernds_cb1@t-online.de>,
<law@redhat.com>, <ebotcazou@libertysurf.fr>,
<steven@gcc.gnu.org>
Subject: [RFC: Patch 4/6 v2] Modify cost model for noce_cmove_arith
Date: Tue, 21 Jun 2016 15:53:00 -0000 [thread overview]
Message-ID: <1466524231-17412-4-git-send-email-james.greenhalgh@arm.com> (raw)
In-Reply-To: <1466524231-17412-1-git-send-email-james.greenhalgh@arm.com>
[-- Attachment #1: Type: text/plain, Size: 1520 bytes --]
Hi,
This patch clears up the cost model for noce_try_cmove_arith. We lose
the "??? FIXME: Magic number 5" comment, and gain a more realistic cost
model for if-converting memory accesses.
This is the patch that has the chance to cause the largest behavioural
changes for most targets - the current heuristic does not take in to
consideration the cost of a conditional move - once we add that the cost
of the converted sequence often looks higher than we allowed before.
I think that missing the cost of the conditional move from these sequences
is not a good idea, and that the cost model should rely on the target giving
back good information. A target that finds tests failing after this patch
should consider either reducing the cost of a conditional move sequence, or
increasing TARGET_MAX_NOCE_IFCVT_SEQ_COST.
As this ups the cost of if-convert dramatically, I've used the new
parameters to ensure that the tests in the testsuite continue to pass on
all targets.
Bootstrapped in series on aarch64 and x86-64.
OK?
Thanks,
James
---
gcc/
2016-06-21 James Greenhalgh <james.greenhalgh@arm.com>
* ifcvt.c (noce_try_cmove_arith): Check costs after constructing
new sequence.
gcc/testsuite/
2016-06-21 James Greenhalgh <james.greenhalgh@arm.com>
* gcc.dg/ifcvt-2.c: Use parameter to guide if-conversion heuristics.
* gcc.dg/ifcvt-3.c: Use parameter to guide if-conversion heuristics.
* gcc.dg/pr68435.c: Use parameter to guide if-conversion heuristics.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0004-RFC-Patch-4-6-v2-Modify-cost-model-for-noce_cmove_ar.patch --]
[-- Type: text/x-patch; name=0004-RFC-Patch-4-6-v2-Modify-cost-model-for-noce_cmove_ar.patch, Size: 3904 bytes --]
diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index f4ad037..78906d3 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -2092,7 +2092,8 @@ noce_try_cmove_arith (struct noce_if_info *if_info)
rtx a = if_info->a;
rtx b = if_info->b;
rtx x = if_info->x;
- rtx orig_a, orig_b;
+ rtx orig_a = a;
+ rtx orig_b = b;
rtx_insn *insn_a, *insn_b;
bool a_simple = if_info->then_simple;
bool b_simple = if_info->else_simple;
@@ -2102,16 +2103,15 @@ noce_try_cmove_arith (struct noce_if_info *if_info)
int is_mem = 0;
enum rtx_code code;
rtx_insn *ifcvt_seq;
+ bool speed_p = optimize_bb_for_speed_p (if_info->test_bb);
/* A conditional move from two memory sources is equivalent to a
conditional on their addresses followed by a load. Don't do this
early because it'll screw alias analysis. Note that we've
already checked for no side effects. */
- /* ??? FIXME: Magic number 5. */
if (cse_not_expected
&& MEM_P (a) && MEM_P (b)
- && MEM_ADDR_SPACE (a) == MEM_ADDR_SPACE (b)
- && noce_estimate_conversion_profitable_p (if_info, 5))
+ && MEM_ADDR_SPACE (a) == MEM_ADDR_SPACE (b))
{
machine_mode address_mode = get_address_mode (a);
@@ -2143,25 +2143,6 @@ noce_try_cmove_arith (struct noce_if_info *if_info)
if (!can_conditionally_move_p (x_mode))
return FALSE;
- unsigned int then_cost;
- unsigned int else_cost;
- if (insn_a)
- then_cost = if_info->then_cost;
- else
- then_cost = 0;
-
- if (insn_b)
- else_cost = if_info->else_cost;
- else
- else_cost = 0;
-
- /* We're going to execute one of the basic blocks anyway, so
- bail out if the most expensive of the two blocks is unacceptable. */
-
- /* TODO: Revisit cost model. */
- if (MAX (then_cost, else_cost) > if_info->max_seq_cost)
- return FALSE;
-
/* Possibly rearrange operands to make things come out more natural. */
if (reversed_comparison_code (if_info->cond, if_info->jump) != UNKNOWN)
{
@@ -2353,6 +2334,12 @@ noce_try_cmove_arith (struct noce_if_info *if_info)
if (!ifcvt_seq)
return FALSE;
+ /* Check that our cost model will allow the transform. */
+
+ if (seq_cost (ifcvt_seq, speed_p) > if_info->max_seq_cost)
+ /* Just return false, the sequence has already been finalized. */
+ return FALSE;
+
emit_insn_before_setloc (ifcvt_seq, if_info->jump,
INSN_LOCATION (if_info->insn_a));
if_info->transform_name = "noce_try_cmove_arith";
diff --git a/gcc/testsuite/gcc.dg/ifcvt-2.c b/gcc/testsuite/gcc.dg/ifcvt-2.c
index e0e1728..73e0dcc 100644
--- a/gcc/testsuite/gcc.dg/ifcvt-2.c
+++ b/gcc/testsuite/gcc.dg/ifcvt-2.c
@@ -1,5 +1,5 @@
/* { dg-do compile { target aarch64*-*-* x86_64-*-* } } */
-/* { dg-options "-fdump-rtl-ce1 -O2" } */
+/* { dg-options "-fdump-rtl-ce1 -O2 --param max-rtl-if-conversion-unpredictable-cost=100" } */
typedef unsigned char uint8_t;
diff --git a/gcc/testsuite/gcc.dg/ifcvt-3.c b/gcc/testsuite/gcc.dg/ifcvt-3.c
index 44233d4..b250bc1 100644
--- a/gcc/testsuite/gcc.dg/ifcvt-3.c
+++ b/gcc/testsuite/gcc.dg/ifcvt-3.c
@@ -1,5 +1,5 @@
/* { dg-do compile { target { { aarch64*-*-* i?86-*-* x86_64-*-* } && lp64 } } } */
-/* { dg-options "-fdump-rtl-ce1 -O2" } */
+/* { dg-options "-fdump-rtl-ce1 -O2 --param max-rtl-if-conversion-unpredictable-cost=100" } */
typedef long long s64;
diff --git a/gcc/testsuite/gcc.dg/pr68435.c b/gcc/testsuite/gcc.dg/pr68435.c
index 765699a..f86b7f8 100644
--- a/gcc/testsuite/gcc.dg/pr68435.c
+++ b/gcc/testsuite/gcc.dg/pr68435.c
@@ -1,5 +1,5 @@
/* { dg-do compile { target aarch64*-*-* x86_64-*-* } } */
-/* { dg-options "-fdump-rtl-ce1 -O2 -w" } */
+/* { dg-options "-fdump-rtl-ce1 -O2 -w --param max-rtl-if-conversion-unpredictable-cost=100" } */
typedef struct cpp_reader cpp_reader;
enum cpp_ttype
next prev parent reply other threads:[~2016-06-21 15:53 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-08 15:01 [Patch] Teach RTL ifcvt to handle multiple simple set instructions James Greenhalgh
2015-09-10 18:24 ` Bernd Schmidt
2015-09-10 21:34 ` Jeff Law
2015-09-11 8:51 ` Kyrill Tkachov
2015-09-11 21:49 ` Jeff Law
2015-09-11 9:04 ` Bernd Schmidt
2015-09-11 9:08 ` Ramana Radhakrishnan
2015-09-11 10:55 ` James Greenhalgh
2015-09-25 15:06 ` [Patch ifcvt costs 0/3] Introduce a new target hook for ifcvt costs James Greenhalgh
2015-09-25 15:06 ` [Patch ifcvt 1/3] Factor out cost calculations from noce cases James Greenhalgh
2015-09-25 15:08 ` [Patch ifcvt 2/3] Move noce_if_info in to ifcvt.h James Greenhalgh
2015-09-25 15:14 ` [Patch Prototype AArch64 ifcvt 4/3] Wire up the new if-convert costs hook for AArch64 James Greenhalgh
2015-09-29 10:43 ` Richard Biener
2015-09-25 15:28 ` [Patch ifcvt 3/3] Create a new target hook for deciding profitability of noce if-conversion James Greenhalgh
2015-09-29 10:36 ` [Patch ifcvt costs 0/3] Introduce a new target hook for ifcvt costs Richard Biener
2015-09-29 15:28 ` James Greenhalgh
2015-09-29 19:52 ` Mike Stump
2015-09-30 8:42 ` Richard Biener
2015-09-30 8:48 ` Richard Biener
2015-09-30 19:01 ` Mike Stump
2015-10-01 9:37 ` Bernd Schmidt
2015-10-09 11:28 ` Bernd Schmidt
2015-10-09 15:28 ` Jeff Law
2016-06-02 16:54 ` [RFC: Patch 0/6] Rewrite the noce-ifcvt cost models James Greenhalgh
2016-06-02 16:54 ` [RFC: Patch 4/6] Modify cost model for noce_cmove_arith James Greenhalgh
2016-06-02 16:54 ` [RFC: Patch 1/6] New target hook: rtx_branch_cost James Greenhalgh
2016-06-03 10:39 ` Richard Biener
2016-06-21 15:51 ` [RFC: Patch 1/6 v2] New target hook: max_noce_ifcvt_seq_cost James Greenhalgh
2016-06-21 15:51 ` [RFC: Patch 5/6 v2] Improve the cost model for multiple-sets James Greenhalgh
2016-07-13 21:23 ` Jeff Law
2016-06-21 15:51 ` [RFC: Patch 3/6 v2] Remove if_info->branch_cost James Greenhalgh
2016-07-13 21:19 ` Jeff Law
2016-06-21 15:51 ` [RFC: Patch 2/6 v2] Factor out the comparisons against magic numbers in ifcvt James Greenhalgh
2016-07-13 21:18 ` Jeff Law
2016-06-21 15:53 ` James Greenhalgh [this message]
2016-07-13 21:22 ` [RFC: Patch 4/6 v2] Modify cost model for noce_cmove_arith Jeff Law
2016-06-21 15:53 ` [RFC: Patch 6/6 v2] Remove second cost model from noce_try_store_flag_mask James Greenhalgh
2016-07-13 21:24 ` Jeff Law
2016-06-21 21:31 ` [RFC: Patch 1/6 v2] New target hook: max_noce_ifcvt_seq_cost Bernhard Reutner-Fischer
2016-06-30 12:01 ` Bernd Schmidt
2016-07-13 21:16 ` Jeff Law
2016-07-20 9:52 ` [Re: RFC: Patch 1/2 v3] " James Greenhalgh
2016-07-20 9:52 ` [Patch RFC: 3/2 v3] Don't expand a conditional move between identical sources James Greenhalgh
2016-07-20 9:53 ` [Patch RFC 4/2 v3] Refactor noce_try_cmove_arith James Greenhalgh
2016-07-20 9:53 ` [RFC: Patch 2/2 v3] Introduce a new cost model for ifcvt James Greenhalgh
2016-07-20 11:41 ` [Re: RFC: Patch 1/2 v3] New target hook: max_noce_ifcvt_seq_cost Bernd Schmidt
2016-07-20 16:40 ` James Greenhalgh
2016-07-21 11:32 ` Bernd Schmidt
2016-06-02 16:55 ` [RFC: Patch 2/6] Factor out the comparisons against magic numbers in ifcvt James Greenhalgh
2016-06-02 16:55 ` [RFC: Patch 5/6] Improve the cost model for multiple-sets James Greenhalgh
2016-06-02 16:56 ` [RFC: Patch 6/6] Remove second cost model from noce_try_store_flag_mask James Greenhalgh
2016-06-02 16:56 ` [RFC: Patch 3/6] Remove if_info->branch_cost James Greenhalgh
2016-06-03 9:32 ` [RFC: Patch 0/6] Rewrite the noce-ifcvt cost models Bernd Schmidt
2016-06-09 16:58 ` Jeff Law
2016-06-10 10:45 ` James Greenhalgh
2015-09-12 14:04 ` [Patch] Teach RTL ifcvt to handle multiple simple set instructions Eric Botcazou
2015-10-30 18:09 ` [Patch ifcvt] " James Greenhalgh
2015-11-04 11:04 ` Bernd Schmidt
2015-11-04 15:37 ` James Greenhalgh
2015-11-06 9:13 ` Christophe Lyon
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=1466524231-17412-4-git-send-email-james.greenhalgh@arm.com \
--to=james.greenhalgh@arm.com \
--cc=Ramana.Radhakrishnan@arm.com \
--cc=bernds_cb1@t-online.de \
--cc=ebotcazou@libertysurf.fr \
--cc=gcc-patches@gcc.gnu.org \
--cc=law@redhat.com \
--cc=nd@arm.com \
--cc=richard.guenther@gmail.com \
--cc=steven@gcc.gnu.org \
/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).