public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r13-1811] PR target/106303: Fix TImode STV related failures on x86.
@ 2022-07-24 11:24 Roger Sayle
0 siblings, 0 replies; only message in thread
From: Roger Sayle @ 2022-07-24 11:24 UTC (permalink / raw)
To: gcc-cvs
https://gcc.gnu.org/g:76d6224b944e1a66bfb1195fb7d35d9726f1aed8
commit r13-1811-g76d6224b944e1a66bfb1195fb7d35d9726f1aed8
Author: Roger Sayle <roger@nextmovesoftware.com>
Date: Sun Jul 24 12:22:22 2022 +0100
PR target/106303: Fix TImode STV related failures on x86.
This patch resolves PR target/106303 (and the related PRs 106347,
106404, 106407) which are ICEs caused by my improvements to x86_64's
128-bit TImode to V1TImode Scalar to Vector (STV) pass. My apologies
for the breakage. The issue is that data flow analysis is used to
partition usage of each TImode pseudo into "chains", where each
chain is analyzed and if suitable converted to vector operations.
The problems appears when some chains for a pseudo are converted,
and others aren't as RTL sharing can result in some mode changes
leaking into other instructions that aren't/shouldn't/can't be
converted, which eventually leads to an ICE for mismatched modes.
My first approach to a fix was to unify more of the STV infrastructure,
reasoning that if TImode STV was exhibiting these problems, but DImode
and SImode STV weren't, the issue was likely to be caused/resolved by
these remaining differences. This appeared to fix some but not all of
the reported PRs. A better solution was then proposed by H.J. Lu in
Bugzilla, that we need to iterate the removal of candidates in the
function timode_remove_non_convertible_regs until there are no further
changes. As each chain is removed from consideration, it in turn may
affect whether other insns/chains can safely be converted.
2022-07-24 Roger Sayle <roger@nextmovesoftware.com>
H.J. Lu <hjl.tools@gmail.com>
gcc/ChangeLog
PR target/106303
PR target/106347
* config/i386/i386-features.cc (make_vector_copies): Move from
general_scalar_chain to scalar_chain.
(convert_reg): Likewise.
(convert_insn_common): New scalar_chain method split out from
general_scalar_chain convert_insn.
(convert_registers): Move from general_scalar_chain to
scalar_chain.
(scalar_chain::convert): Call convert_insn_common before calling
convert_insn.
(timode_remove_non_convertible_regs): Iterate until there are
no further changes to the candidates.
* config/i386/i386-features.h (scalar_chain::hash_map): Move
from general_scalar_chain.
(scalar_chain::convert_reg): Likewise.
(scalar_chain::convert_insn_common): New shared method.
(scalar_chain::make_vector_copies): Move from general_scalar_chain.
(scalar_chain::convert_registers): Likewise. No longer virtual.
(general_scalar_chain::hash_map): Delete. Moved to scalar_chain.
(general_scalar_chain::convert_reg): Likewise.
(general_scalar_chain::make_vector_copies): Likewise.
(general_scalar_chain::convert_registers): Delete virtual method.
(timode_scalar_chain::convert_registers): Likewise.
gcc/testsuite/ChangeLog
PR target/106303
PR target/106347
* gcc.target/i386/pr106303.c: New test case.
* gcc.target/i386/pr106347.c: New test case.
Diff:
---
gcc/config/i386/i386-features.cc | 116 +++++++++++++++++--------------
gcc/config/i386/i386-features.h | 12 ++--
gcc/testsuite/gcc.target/i386/pr106303.c | 25 +++++++
gcc/testsuite/gcc.target/i386/pr106347.c | 24 +++++++
4 files changed, 119 insertions(+), 58 deletions(-)
diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc
index 813b2032925..aa5de714edb 100644
--- a/gcc/config/i386/i386-features.cc
+++ b/gcc/config/i386/i386-features.cc
@@ -708,7 +708,7 @@ gen_gpr_to_xmm_move_src (enum machine_mode vmode, rtx gpr)
and replace its uses in a chain. */
void
-general_scalar_chain::make_vector_copies (rtx_insn *insn, rtx reg)
+scalar_chain::make_vector_copies (rtx_insn *insn, rtx reg)
{
rtx vreg = *defs_map.get (reg);
@@ -772,7 +772,7 @@ general_scalar_chain::make_vector_copies (rtx_insn *insn, rtx reg)
scalar uses outside of the chain. */
void
-general_scalar_chain::convert_reg (rtx_insn *insn, rtx dst, rtx src)
+scalar_chain::convert_reg (rtx_insn *insn, rtx dst, rtx src)
{
start_sequence ();
if (!TARGET_INTER_UNIT_MOVES_FROM_VEC)
@@ -973,10 +973,10 @@ scalar_chain::convert_compare (rtx op1, rtx op2, rtx_insn *insn)
UNSPEC_PTEST);
}
-/* Convert INSN to vector mode. */
+/* Helper function for converting INSN to vector mode. */
void
-general_scalar_chain::convert_insn (rtx_insn *insn)
+scalar_chain::convert_insn_common (rtx_insn *insn)
{
/* Generate copies for out-of-chain uses of defs and adjust debug uses. */
for (df_ref ref = DF_INSN_DEFS (insn); ref; ref = DF_REF_NEXT_LOC (ref))
@@ -1037,7 +1037,13 @@ general_scalar_chain::convert_insn (rtx_insn *insn)
XEXP (note, 0) = *vreg;
*DF_REF_REAL_LOC (ref) = *vreg;
}
+}
+
+/* Convert INSN to vector mode. */
+void
+general_scalar_chain::convert_insn (rtx_insn *insn)
+{
rtx def_set = single_set (insn);
rtx src = SET_SRC (def_set);
rtx dst = SET_DEST (def_set);
@@ -1475,7 +1481,7 @@ timode_scalar_chain::convert_insn (rtx_insn *insn)
Also populates defs_map which is used later by convert_insn. */
void
-general_scalar_chain::convert_registers ()
+scalar_chain::convert_registers ()
{
bitmap_iterator bi;
unsigned id;
@@ -1510,7 +1516,9 @@ scalar_chain::convert ()
EXECUTE_IF_SET_IN_BITMAP (insns, 0, id, bi)
{
- convert_insn (DF_INSN_UID_GET (id)->insn);
+ rtx_insn *insn = DF_INSN_UID_GET (id)->insn;
+ convert_insn_common (insn);
+ convert_insn (insn);
converted_insns++;
}
@@ -1843,56 +1851,62 @@ timode_remove_non_convertible_regs (bitmap candidates)
bitmap_iterator bi;
unsigned id;
bitmap regs = BITMAP_ALLOC (NULL);
+ bool changed;
- EXECUTE_IF_SET_IN_BITMAP (candidates, 0, id, bi)
- {
- rtx def_set = single_set (DF_INSN_UID_GET (id)->insn);
- rtx dest = SET_DEST (def_set);
- rtx src = SET_SRC (def_set);
-
- if ((!REG_P (dest)
- || bitmap_bit_p (regs, REGNO (dest))
- || HARD_REGISTER_P (dest))
- && (!REG_P (src)
- || bitmap_bit_p (regs, REGNO (src))
- || HARD_REGISTER_P (src)))
- continue;
-
- if (REG_P (dest))
- timode_check_non_convertible_regs (candidates, regs,
- REGNO (dest));
-
- if (REG_P (src))
- timode_check_non_convertible_regs (candidates, regs,
- REGNO (src));
- }
+ do {
+ changed = false;
+ EXECUTE_IF_SET_IN_BITMAP (candidates, 0, id, bi)
+ {
+ rtx def_set = single_set (DF_INSN_UID_GET (id)->insn);
+ rtx dest = SET_DEST (def_set);
+ rtx src = SET_SRC (def_set);
+
+ if ((!REG_P (dest)
+ || bitmap_bit_p (regs, REGNO (dest))
+ || HARD_REGISTER_P (dest))
+ && (!REG_P (src)
+ || bitmap_bit_p (regs, REGNO (src))
+ || HARD_REGISTER_P (src)))
+ continue;
+
+ if (REG_P (dest))
+ timode_check_non_convertible_regs (candidates, regs,
+ REGNO (dest));
+
+ if (REG_P (src))
+ timode_check_non_convertible_regs (candidates, regs,
+ REGNO (src));
+ }
- EXECUTE_IF_SET_IN_BITMAP (regs, 0, id, bi)
- {
- for (df_ref def = DF_REG_DEF_CHAIN (id);
- def;
- def = DF_REF_NEXT_REG (def))
- if (bitmap_bit_p (candidates, DF_REF_INSN_UID (def)))
- {
- if (dump_file)
- fprintf (dump_file, "Removing insn %d from candidates list\n",
- DF_REF_INSN_UID (def));
+ EXECUTE_IF_SET_IN_BITMAP (regs, 0, id, bi)
+ {
+ for (df_ref def = DF_REG_DEF_CHAIN (id);
+ def;
+ def = DF_REF_NEXT_REG (def))
+ if (bitmap_bit_p (candidates, DF_REF_INSN_UID (def)))
+ {
+ if (dump_file)
+ fprintf (dump_file, "Removing insn %d from candidates list\n",
+ DF_REF_INSN_UID (def));
- bitmap_clear_bit (candidates, DF_REF_INSN_UID (def));
- }
+ bitmap_clear_bit (candidates, DF_REF_INSN_UID (def));
+ changed = true;
+ }
- for (df_ref ref = DF_REG_USE_CHAIN (id);
- ref;
- ref = DF_REF_NEXT_REG (ref))
- if (bitmap_bit_p (candidates, DF_REF_INSN_UID (ref)))
- {
- if (dump_file)
- fprintf (dump_file, "Removing insn %d from candidates list\n",
- DF_REF_INSN_UID (ref));
+ for (df_ref ref = DF_REG_USE_CHAIN (id);
+ ref;
+ ref = DF_REF_NEXT_REG (ref))
+ if (bitmap_bit_p (candidates, DF_REF_INSN_UID (ref)))
+ {
+ if (dump_file)
+ fprintf (dump_file, "Removing insn %d from candidates list\n",
+ DF_REF_INSN_UID (ref));
- bitmap_clear_bit (candidates, DF_REF_INSN_UID (ref));
- }
- }
+ bitmap_clear_bit (candidates, DF_REF_INSN_UID (ref));
+ changed = true;
+ }
+ }
+ } while (changed);
BITMAP_FREE (regs);
}
diff --git a/gcc/config/i386/i386-features.h b/gcc/config/i386/i386-features.h
index 88b222debc9..3d88a88e014 100644
--- a/gcc/config/i386/i386-features.h
+++ b/gcc/config/i386/i386-features.h
@@ -149,6 +149,7 @@ class scalar_chain
bitmap defs_conv;
bitmap insns_conv;
+ hash_map<rtx, rtx> defs_map;
unsigned n_sse_to_integer;
unsigned n_integer_to_sse;
@@ -161,12 +162,15 @@ class scalar_chain
void emit_conversion_insns (rtx insns, rtx_insn *pos);
rtx convert_compare (rtx op1, rtx op2, rtx_insn *insn);
void mark_dual_mode_def (df_ref def);
+ void convert_reg (rtx_insn *insn, rtx dst, rtx src);
+ void convert_insn_common (rtx_insn *insn);
+ void make_vector_copies (rtx_insn *, rtx);
+ void convert_registers ();
private:
void add_insn (bitmap candidates, unsigned insn_uid);
void analyze_register_chain (bitmap candidates, df_ref ref);
virtual void convert_insn (rtx_insn *insn) = 0;
- virtual void convert_registers () = 0;
virtual void convert_op (rtx *op, rtx_insn *insn) = 0;
};
@@ -178,12 +182,8 @@ class general_scalar_chain : public scalar_chain
int compute_convert_gain () final override;
private:
- hash_map<rtx, rtx> defs_map;
void convert_insn (rtx_insn *insn) final override;
- void convert_reg (rtx_insn *insn, rtx dst, rtx src);
void convert_op (rtx *op, rtx_insn *insn);
- void make_vector_copies (rtx_insn *, rtx);
- void convert_registers () final override;
int vector_const_cost (rtx exp);
};
@@ -197,8 +197,6 @@ class timode_scalar_chain : public scalar_chain
void fix_debug_reg_uses (rtx reg);
void convert_insn (rtx_insn *insn) final override;
void convert_op (rtx *op, rtx_insn *insn);
- /* We don't convert registers to different size. */
- void convert_registers () final override {}
};
} // anon namespace
diff --git a/gcc/testsuite/gcc.target/i386/pr106303.c b/gcc/testsuite/gcc.target/i386/pr106303.c
new file mode 100644
index 00000000000..19cce404fa4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr106303.c
@@ -0,0 +1,25 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-inline-small-functions" } */
+
+struct a {
+ int b;
+ int c;
+ int d;
+ int e;
+} i, j;
+int f, g, h;
+struct a k() {
+ while (f)
+ i = j;
+ if (g) {
+ for (; h; h++)
+ i = j;
+ return j;
+ }
+ return i;
+}
+int main() {
+ k();
+ return 0;
+}
+
diff --git a/gcc/testsuite/gcc.target/i386/pr106347.c b/gcc/testsuite/gcc.target/i386/pr106347.c
new file mode 100644
index 00000000000..003dd1b6bf2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr106347.c
@@ -0,0 +1,24 @@
+/* { dg-do compile { target int128 } } */
+/* { dg-options "-O2 -fno-expensive-optimizations" } */
+
+__int128 m;
+int n;
+
+__attribute__ ((noinline)) int
+return_zero (void)
+{
+ return 0;
+}
+
+void
+foo (void)
+{
+ while (m < 0)
+ {
+ if (n || return_zero ())
+ __builtin_trap ();
+
+ ++m;
+ }
+}
+
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2022-07-24 11:24 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-24 11:24 [gcc r13-1811] PR target/106303: Fix TImode STV related failures on x86 Roger Sayle
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).