public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Robin Dapp <rdapp@linux.ibm.com>
To: gcc-patches@gcc.gnu.org, richard.sandiford@arm.com
Subject: [PATCH 3/7] ifcvt: Improve costs handling for noce_convert_multiple.
Date: Fri, 25 Jun 2021 18:09:01 +0200	[thread overview]
Message-ID: <20210625160905.23786-4-rdapp@linux.ibm.com> (raw)
In-Reply-To: <20210625160905.23786-1-rdapp@linux.ibm.com>

When noce_convert_multiple is called the original costs are not yet
initialized.  Therefore, up to now, costs were only ever unfairly
compared against COSTS_N_INSNS (2).  This would lead to
default_noce_conversion_profitable_p () rejecting all but the most
contrived of sequences.

This patch temporarily initialized the original costs by counting
a compare and all the sets inside the then_bb.
---
 gcc/ifcvt.c | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index 6006055f26a..ac0c142c9fe 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -3382,14 +3382,17 @@ noce_convert_multiple_sets (struct noce_if_info *if_info)
    (SET (REG) (REG)) insns suitable for conversion to a series
    of conditional moves.  Also check that we have more than one set
    (other routines can handle a single set better than we would), and
-   fewer than PARAM_MAX_RTL_IF_CONVERSION_INSNS sets.  */
+   fewer than PARAM_MAX_RTL_IF_CONVERSION_INSNS sets.  While going
+   through the insns store the sum of their potential costs in COST.  */
 
 static bool
-bb_ok_for_noce_convert_multiple_sets (basic_block test_bb)
+bb_ok_for_noce_convert_multiple_sets (basic_block test_bb, unsigned *cost)
 {
   rtx_insn *insn;
   unsigned count = 0;
   unsigned param = param_max_rtl_if_conversion_insns;
+  bool speed_p = optimize_bb_for_speed_p (test_bb);
+  unsigned potential_cost = 0;
 
   FOR_BB_INSNS (test_bb, insn)
     {
@@ -3425,9 +3428,13 @@ bb_ok_for_noce_convert_multiple_sets (basic_block test_bb)
       if (!can_conditionally_move_p (GET_MODE (dest)))
 	return false;
 
+      potential_cost += pattern_cost (set, speed_p);
+
       count++;
     }
 
+  *cost += potential_cost;
+
   /* If we would only put out one conditional move, the other strategies
      this pass tries are better optimized and will be more appropriate.
      Some targets want to strictly limit the number of conditional moves
@@ -3475,11 +3482,23 @@ noce_process_if_block (struct noce_if_info *if_info)
      to calculate a value for x.
      ??? For future expansion, further expand the "multiple X" rules.  */
 
-  /* First look for multiple SETS.  */
+  /* First look for multiple SETS.  The original costs already
+     include a compare that we will be needing either way.  When
+     comparing costs we want to use the branch instruction cost and
+     the sets vs. the cmovs generated here.  Therefore subtract
+     the costs of the compare before checking.
+     ??? Actually, instead of the branch instruction costs we might want
+     to use COSTS_N_INSNS (BRANCH_COST ()) as in other places.*/
+
+  unsigned potential_cost = if_info->original_cost - COSTS_N_INSNS (1);
+  unsigned old_cost = if_info->original_cost;
   if (!else_bb
       && HAVE_conditional_move
-      && bb_ok_for_noce_convert_multiple_sets (then_bb))
+      && bb_ok_for_noce_convert_multiple_sets (then_bb, &potential_cost))
     {
+      /* Temporarily set the original costs to what we estimated so
+	 we can determine if the transformation is worth it.  */
+      if_info->original_cost = potential_cost;
       if (noce_convert_multiple_sets (if_info))
 	{
 	  if (dump_file && if_info->transform_name)
@@ -3487,6 +3506,9 @@ noce_process_if_block (struct noce_if_info *if_info)
 		     if_info->transform_name);
 	  return TRUE;
 	}
+
+      /* Restore the original costs.  */
+      if_info->original_cost = old_cost;
     }
 
   bool speed_p = optimize_bb_for_speed_p (test_bb);
-- 
2.31.1


  parent reply	other threads:[~2021-06-25 16:09 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-25 16:08 [PATCH 0/7] ifcvt: Convert multiple Robin Dapp
2021-06-25 16:08 ` [PATCH 1/7] ifcvt: Check if cmovs are needed Robin Dapp
2021-07-15 20:10   ` Richard Sandiford
2021-07-22 12:06     ` Robin Dapp
2021-07-26 19:08       ` Richard Sandiford
2021-09-15  8:39         ` Robin Dapp
2021-10-14  8:45           ` Richard Sandiford
2021-10-14 14:20             ` Robin Dapp
2021-10-14 14:32               ` Richard Sandiford
2021-10-18 11:40                 ` Robin Dapp
2021-11-03  8:55                   ` Robin Dapp
2021-11-05 15:33                   ` Richard Sandiford
2021-11-12 13:00                     ` Robin Dapp
2021-11-30 16:36                       ` Richard Sandiford
2021-06-25 16:09 ` [PATCH 2/7] ifcvt: Allow constants for noce_convert_multiple Robin Dapp
2021-07-15 20:25   ` Richard Sandiford
2021-06-25 16:09 ` Robin Dapp [this message]
2021-07-15 20:42   ` [PATCH 3/7] ifcvt: Improve costs handling " Richard Sandiford
2021-07-22 12:07     ` Robin Dapp
2021-07-26 19:10       ` Richard Sandiford
2021-06-25 16:09 ` [PATCH 4/7] ifcvt/optabs: Allow using a CC comparison for emit_conditional_move Robin Dapp
2021-07-15 20:54   ` Richard Sandiford
2021-07-22 12:07     ` Robin Dapp
2021-07-26 19:31       ` Richard Sandiford
2021-07-27 20:49         ` Robin Dapp
2021-08-06 12:14           ` Richard Sandiford
2021-06-25 16:09 ` [PATCH 5/7] ifcvt: Try re-using CC for conditional moves Robin Dapp
2021-07-22 12:12   ` Robin Dapp
2021-06-25 16:09 ` [PATCH 6/7] testsuite/s390: Add tests for noce_convert_multiple Robin Dapp
2021-06-25 16:09 ` [PATCH 7/7] s390: Increase costs for load on condition and change movqicc expander Robin Dapp
2021-07-13 12:42 ` [PATCH 0/7] ifcvt: Convert multiple Robin Dapp

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=20210625160905.23786-4-rdapp@linux.ibm.com \
    --to=rdapp@linux.ibm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=richard.sandiford@arm.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).