From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1251) id 5A9D43858C39; Sun, 24 Jul 2022 11:24:23 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5A9D43858C39 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Roger Sayle To: gcc-cvs@gcc.gnu.org Subject: [gcc r13-1811] PR target/106303: Fix TImode STV related failures on x86. X-Act-Checkin: gcc X-Git-Author: Roger Sayle X-Git-Refname: refs/heads/master X-Git-Oldrev: 0e6fa997306dd03bd1a019b4f4d837cb2ff8daf8 X-Git-Newrev: 76d6224b944e1a66bfb1195fb7d35d9726f1aed8 Message-Id: <20220724112423.5A9D43858C39@sourceware.org> Date: Sun, 24 Jul 2022 11:24:23 +0000 (GMT) X-BeenThere: gcc-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 24 Jul 2022 11:24:23 -0000 https://gcc.gnu.org/g:76d6224b944e1a66bfb1195fb7d35d9726f1aed8 commit r13-1811-g76d6224b944e1a66bfb1195fb7d35d9726f1aed8 Author: Roger Sayle 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 H.J. Lu 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 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 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; + } +} +