From: "Kewen.Lin" <linkw@linux.ibm.com>
To: GCC Patches <gcc-patches@gcc.gnu.org>
Cc: Segher Boessenkool <segher@kernel.crashing.org>,
Bill Schmidt <wschmidt@linux.ibm.com>
Subject: [PATCH/RFC] combine: Tweak the condition of last_set invalidation
Date: Wed, 16 Dec 2020 16:49:49 +0800 [thread overview]
Message-ID: <6bcd32fa-d0ef-b136-ddd9-92a1d21f60af@linux.ibm.com> (raw)
[-- Attachment #1: Type: text/plain, Size: 2076 bytes --]
Hi,
When I was investigating unsigned int vec_init issue on Power,
I happened to find there seems something we can enhance in how
combine pass invalidate last_set (set last_set_invalid nonzero).
Currently we have the check:
if (!insn
|| (value && rsp->last_set_table_tick >= label_tick_ebb_start))
rsp->last_set_invalid = 1;
which means if we want to record some value for some reg and
this reg got refered before in a valid scope, we invalidate the
set of reg (last_set_invalid to 1). It avoids to find the wrong
set for one reg reference, such as the case like:
... op regX // this regX could find wrong last_set below
regX = ... // if we think this set is valid
... op regX
But because of retry's existence, the last_set_table_tick could
be set by some later reference insns, but we see it's set due
to retry on the set (for that reg) insn again, such as:
insn 1
insn 2
regX = ... --> (a)
... op regX --> (b)
insn 3
// assume all in the same BB.
Assuming we combine 1, 2 -> 3 sucessfully and replace them as two
(3 insns -> 2 insns), retrying from insn1 or insn2 again:
it will scan insn (a) again, the below condition holds for regX:
(value && rsp->last_set_table_tick >= label_tick_ebb_start)
it will mark this set as invalid set. But actually the
last_set_table_tick here is set by insn (b) before retrying, so it
should be safe to be taken as valid set.
This proposal is to check whether the last_set_table safely happens
after the current set, make the set still valid if so.
Bootstrapped/regtested on powerpc64le-linux-gnu (P9),
aarch64-linux-gnu and x86_64-pc-linux-gnu.
Full SPEC2017 building shows this patch gets more sucessful combines
from 1902208 to 1902243 (trivial though).
Any comments are highly appreciated!
BR,
Kewen
-----
gcc/ChangeLog:
* combine.c (struct reg_stat_type): New member
last_set_table_luid.
(update_table_tick): Add one argument for insn luid and
set last_set_table_luid with it.
(record_value_for_reg): Adjust the condition to set
last_set_invalid nonzero.
[-- Attachment #2: last_set_luid.diff --]
[-- Type: text/plain, Size: 3570 bytes --]
diff --git a/gcc/combine.c b/gcc/combine.c
index 6fb2fa82c3f..2f45a0ad733 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -202,6 +202,10 @@ struct reg_stat_type {
int last_set_table_tick;
+ /* Record the luid of the insn whose expression involving register n. */
+
+ int last_set_table_luid;
+
/* Record the value of label_tick when the value for register n is placed in
last_set_value. */
@@ -480,7 +484,7 @@ static rtx gen_lowpart_for_combine (machine_mode, rtx);
static enum rtx_code simplify_compare_const (enum rtx_code, machine_mode,
rtx, rtx *);
static enum rtx_code simplify_comparison (enum rtx_code, rtx *, rtx *);
-static void update_table_tick (rtx);
+static void update_table_tick (rtx, int);
static void record_value_for_reg (rtx, rtx_insn *, rtx);
static void check_promoted_subreg (rtx_insn *, rtx);
static void record_dead_and_set_regs_1 (rtx, const_rtx, void *);
@@ -13228,7 +13232,7 @@ count_rtxs (rtx x)
for each register mentioned. Similar to mention_regs in cse.c */
static void
-update_table_tick (rtx x)
+update_table_tick (rtx x, int insn_luid)
{
enum rtx_code code = GET_CODE (x);
const char *fmt = GET_RTX_FORMAT (code);
@@ -13243,7 +13247,21 @@ update_table_tick (rtx x)
for (r = regno; r < endregno; r++)
{
reg_stat_type *rsp = ®_stat[r];
- rsp->last_set_table_tick = label_tick;
+ if (rsp->last_set_table_tick >= label_tick_ebb_start)
+ {
+ /* Later references should not have lower ticks. */
+ gcc_assert (label_tick >= rsp->last_set_table_tick);
+ /* Should pick up the lowest luid if the references
+ are in the same block. */
+ if (label_tick == rsp->last_set_table_tick
+ && rsp->last_set_table_luid > insn_luid)
+ rsp->last_set_table_luid = insn_luid;
+ }
+ else
+ {
+ rsp->last_set_table_tick = label_tick;
+ rsp->last_set_table_luid = insn_luid;
+ }
}
return;
@@ -13279,16 +13297,17 @@ update_table_tick (rtx x)
if (ARITHMETIC_P (x0)
&& (x1 == XEXP (x0, 0) || x1 == XEXP (x0, 1)))
{
- update_table_tick (XEXP (x0, x1 == XEXP (x0, 0) ? 1 : 0));
+ update_table_tick (XEXP (x0, x1 == XEXP (x0, 0) ? 1 : 0),
+ insn_luid);
break;
}
}
- update_table_tick (XEXP (x, i));
+ update_table_tick (XEXP (x, i), insn_luid);
}
else if (fmt[i] == 'E')
for (j = 0; j < XVECLEN (x, i); j++)
- update_table_tick (XVECEXP (x, i, j));
+ update_table_tick (XVECEXP (x, i, j), insn_luid);
}
/* Record that REG is set to VALUE in insn INSN. If VALUE is zero, we
@@ -13359,7 +13378,10 @@ record_value_for_reg (rtx reg, rtx_insn *insn, rtx value)
/* Mark registers that are being referenced in this value. */
if (value)
- update_table_tick (value);
+ {
+ gcc_assert (insn);
+ update_table_tick (value, DF_INSN_LUID (insn));
+ }
/* Now update the status of each register being set.
If someone is using this register in this block, set this register
@@ -13372,8 +13394,11 @@ record_value_for_reg (rtx reg, rtx_insn *insn, rtx value)
{
rsp = ®_stat[i];
rsp->last_set_label = label_tick;
+ gcc_assert (label_tick >= rsp->last_set_table_tick);
if (!insn
- || (value && rsp->last_set_table_tick >= label_tick_ebb_start))
+ || (value && rsp->last_set_table_tick >= label_tick_ebb_start
+ && !(label_tick == rsp->last_set_table_tick
+ && DF_INSN_LUID (insn) < rsp->last_set_table_luid)))
rsp->last_set_invalid = 1;
else
rsp->last_set_invalid = 0;
next reply other threads:[~2020-12-16 8:49 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-16 8:49 Kewen.Lin [this message]
2021-01-14 2:29 ` PING^1 " Kewen.Lin
2021-01-15 0:22 ` Segher Boessenkool
2021-01-15 8:06 ` Kewen.Lin
2021-01-22 0:30 ` Segher Boessenkool
2021-01-22 2:21 ` Kewen.Lin
2021-05-07 2:45 ` Kewen.Lin
2021-05-26 3:04 ` PING^2 " Kewen.Lin
2021-06-09 2:32 ` PING^3 " Kewen.Lin
2021-06-09 20:17 ` Segher Boessenkool
2021-06-11 13:16 ` [PATCH v2] " Kewen.Lin
2021-06-28 7:00 ` PING^1 " Kewen.Lin
2021-07-15 2:00 ` PING^2 " Kewen.Lin
2021-09-08 7:03 ` PING^3 " Kewen.Lin
2021-10-13 2:27 ` PING^4 " Kewen.Lin
2021-10-20 9:28 ` PING^5 " Kewen.Lin
2021-11-04 10:56 ` PING^6 " Kewen.Lin
2021-11-22 2:22 ` PING^7 " Kewen.Lin
2021-11-29 22:28 ` Segher Boessenkool
2021-11-30 8:47 ` 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=6bcd32fa-d0ef-b136-ddd9-92a1d21f60af@linux.ibm.com \
--to=linkw@linux.ibm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=segher@kernel.crashing.org \
--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).