From patchwork Sat Jul 23 07:32:14 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roger Sayle X-Patchwork-Id: 1659814 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=nextmovesoftware.com header.i=@nextmovesoftware.com header.a=rsa-sha256 header.s=default header.b=csh1UcGq; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4LqdLg4wJ3z9tB1 for ; Sat, 23 Jul 2022 17:32:38 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A2B383857425 for ; Sat, 23 Jul 2022 07:32:32 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from server.nextmovesoftware.com (server.nextmovesoftware.com [162.254.253.69]) by sourceware.org (Postfix) with ESMTPS id 7DB39385828D for ; Sat, 23 Jul 2022 07:32:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7DB39385828D Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=nextmovesoftware.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=nextmovesoftware.com DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=nextmovesoftware.com; s=default; h=Content-Type:MIME-Version:Message-ID: Date:Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:In-Reply-To:References:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=rtPzAkUeYrj4wOO/X3101YUuHJYVA8nveawBwuNyRxY=; b=csh1UcGqY8OhdsI4B+bh0lTSqe Remp99l08/qdvHLN9TgClasuz1yUsN35TFvCUyWzLKRVsUC+sW0eUVM0Syo3tWA3Pd5macQSp2ebH Yu61TPbS2vSQTfyy32eJ9DUW8PNM7JFh9XQCops6eR2OkS3QEGgCvcXzYZQrGtt5+LB/IZRCUZ5Qt hP+XlgjLK0yg+mows7AydQKvLO1X1AF3QgvFW5MyMeEGFNuRdkhIQUCEyEOUx7x6sNtTWgIPtS5pd rWW+n6a8fkuEX6RBo1YqpcnT/YDFXucx3F/U8ZzkZiAn8YLz8HnpBIwjAromvfqdKGF/3BA+qXcmY sX4YyDVA==; Received: from host109-154-33-170.range109-154.btcentralplus.com ([109.154.33.170]:55234 helo=Dell) by server.nextmovesoftware.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1oF9cY-0007m9-E1; Sat, 23 Jul 2022 03:32:18 -0400 From: "Roger Sayle" To: "'GCC Patches'" Subject: [x86 PATCH] PR target/106303: Fix TImode STV related failures. Date: Sat, 23 Jul 2022 08:32:14 +0100 Message-ID: <051501d89e66$55fcbff0$01f63fd0$@nextmovesoftware.com> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AdieY7iQOu8ZEz9lRJqYJtwBZV/I8g== Content-Language: en-gb X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - server.nextmovesoftware.com X-AntiAbuse: Original Domain - gcc.gnu.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - nextmovesoftware.com X-Get-Message-Sender-Via: server.nextmovesoftware.com: authenticated_id: roger@nextmovesoftware.com X-Authenticated-Sender: server.nextmovesoftware.com: roger@nextmovesoftware.com X-Source: X-Source-Args: X-Source-Dir: X-Spam-Status: No, score=-10.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_BARRACUDACENTRAL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" 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 (thanks!) 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. This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and make -k check, both with and without --target_board=unix{-m32}, with no new failures. Ok for mainline? 2022-07-23 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. Thanks in advance (and sorry again for the inconvenience), Roger diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc index 813b203..aa5de71 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 88b222d..3d88a88 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 0000000..19cce40 --- /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 0000000..003dd1b --- /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; + } +} +