From patchwork Fri Jan 21 14:59:07 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roger Sayle X-Patchwork-Id: 50310 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 824FC385780A for ; Fri, 21 Jan 2022 14:59:27 +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 DCC123858C60 for ; Fri, 21 Jan 2022 14:59:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DCC123858C60 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:To:From:Sender:Reply-To:Cc: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=h9oeD2RmqJ6HQe/aq9LfVkp4Bp7vnuZ8RoA/C8VpKbE=; b=klAZjHVrw/k95s2zri55MRV61f EiEkVWCfY5LLuxvlVcolDY4dj14bUAThvFznnwzGej7uo92RS2RfnSegyB5Qjq5tnoXP7LPYSknvu AweoPaZSDW9Dybh5NNBHRwX/LPGgD/f2s6CLPAAY3iDhA+vszJfi6QWp2tb9dSQBY1+k84hEdd0qD 1YjjoRx3qd8EREKGEY33Dc9Xzu71B+H0hVVRI438jsGkIRO9uHX6dUcvDzedHN9FC+N+R3FP3XXtF 3FyqPPX4unqMZgkQDpD/EF08hhlmjkhc8RF4tqINV43JIWT1kAB6xQ8wbmLZwLttIH3sjF18FujC8 8Kqy/gLg==; Received: from [185.62.158.67] (port=58619 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 1nAvNb-0001X8-Sn for gcc-patches@gcc.gnu.org; Fri, 21 Jan 2022 09:59:08 -0500 From: "Roger Sayle" To: "'GCC Patches'" Subject: [PATCH] PR middle-end/104140: bootstrap ICE on riscv. Date: Fri, 21 Jan 2022 14:59:07 -0000 Message-ID: <006001d80ed7$710cc750$532655f0$@nextmovesoftware.com> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AdgO1xttRsevglU3SViS6+HNFpJPZA== 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=-12.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, HTML_MESSAGE, KAM_SHORT, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-Content-Filtered-By: Mailman/MimeDel 2.1.29 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+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" This patch resolves the P1 "ice-on-valid-code" regression boostrapping GCC on risv-unknown-linux-gnu caused by my recent MULT_HIGHPART_EXPR functionality. RISC-V differs from x86_64 and many targets by supporting a usmusidi3 instruction, basically a widening multiply where one operand is signed and the other is unsigned. Alas the final version of my patch to recognize MULT_HIGHPART_EXPR didn't sufficiently defend against the operands of WIDEN_MULT_EXPR having different signedness. This is fixed by the two-line change to tree-ssa-math-opts.c's convert_mult_to_highpart in the patch below. The majority of the rest of the patch is to the documentation (in tree.def and generic.texi). It turns out that WIDEN_MULT_EXPR wasn't previously documented in generic.texi, let alone the slightly unusual semantics of allowing mismatched (signed vs unsigned) operands. This also clarifies that MULT_HIGHPART_EXPR currently requires the signedness of operands to match [but this might change in future release of GCC to support targets with usmul3_highpart. The one final chunk of this patch (that is hopefully sufficiently close to obvious for stage 4) is a similar (NULL pointer) sanity check in riscv_cpu_cpp_builtins. Currently running cc1 from the command line (or from gdb) without specifying -march results in a segmentation fault (ICE). This is a minor annoyance tracking down issues (in cross compilers) for riscv, and trivially fixed as below. This patch has been tested both on x86_64-pc-linux-gnu with a full make bootstrap and make -k check, and on a cross-compiler to riscv-unknown-linux-gnu where I was able to confirm the new test case now passes. Ok for mainline? 2022-01-22 Roger Sayle gcc/ChangeLog * tree-ssa-math-opts.c (convert_mult_to_highpart): Check that the operands of the widening multiplication are either both signed or both unsigned, and abort the conversion if mismatched. * doc/generic.texi (WIDEN_MULT_EXPR): Describe expression node. (MULT_HIGHPART_EXPR): Clarify that operands must have the same signedness. * tree.def (MULT_HIGHPART_EXPR): Document both operands must have integer types with the same precision and signedness. (WIDEN_MULT_EXPR): Document that operands must have integer types with the same precision, but possibly differing signedness. * config/riscv/risc-v.c (riscv_cpu_cpp_builtins): Defend against riscv_current_subset_list returning a NULL pointer (empty list). gcc/testsuite/ChangeLog * gcc.target/riscv/pr104140.c: New test case. Thanks in advance (and sorry for the inconvenience). Roger diff --git a/gcc/config/riscv/riscv-c.c b/gcc/config/riscv/riscv-c.c index 211472f..73c62f4 100644 --- a/gcc/config/riscv/riscv-c.c +++ b/gcc/config/riscv/riscv-c.c @@ -108,6 +108,9 @@ riscv_cpu_cpp_builtins (cpp_reader *pfile) builtin_define_with_int_value ("__riscv_arch_test", 1); const riscv_subset_list *subset_list = riscv_current_subset_list (); + if (!subset_list) + return; + size_t max_ext_len = 0; /* Figure out the max length of extension name for reserving buffer. */ diff --git a/gcc/doc/generic.texi b/gcc/doc/generic.texi index bb07775..bf27a0e 100644 --- a/gcc/doc/generic.texi +++ b/gcc/doc/generic.texi @@ -1318,6 +1318,7 @@ The type of the node specifies the alignment of the access. @tindex PLUS_EXPR @tindex MINUS_EXPR @tindex MULT_EXPR +@tindex WIDEN_MULT_EXPR @tindex MULT_HIGHPART_EXPR @tindex RDIV_EXPR @tindex TRUNC_DIV_EXPR @@ -1532,10 +1533,18 @@ one operand is of floating type and the other is of integral type. The behavior of these operations on signed arithmetic overflow is controlled by the @code{flag_wrapv} and @code{flag_trapv} variables. +@item WIDEN_MULT_EXPR +This node represents a widening multiplication. The operands have +integral types with same @var{b} bits of precision, producing an +integral type result with at least @math{2@var{b}} bits of precision. +The behaviour is equivalent to extending both operands, possibly of +different signedness, to the result type, then multiplying them. + @item MULT_HIGHPART_EXPR This node represents the ``high-part'' of a widening multiplication. For an integral type with @var{b} bits of precision, the result is the most significant @var{b} bits of the full @math{2@var{b}} product. +Both operands must have the same precision and same signedness. @item RDIV_EXPR This node represents a floating point division operation. diff --git a/gcc/testsuite/gcc.target/riscv/pr104140.c b/gcc/testsuite/gcc.target/riscv/pr104140.c new file mode 100644 index 0000000..648e131 --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/pr104140.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -march=rv32im -mabi=ilp32" } */ +int x; +unsigned u, v; +void f (void) +{ + long long y = x; + u = y * v >> 32; +} +void g (void) { f (); } + diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c index 1b6a57b..28ed116 100644 --- a/gcc/tree-ssa-math-opts.c +++ b/gcc/tree-ssa-math-opts.c @@ -4608,6 +4608,10 @@ convert_mult_to_highpart (gassign *stmt, gimple_stmt_iterator *gsi) if (bits < prec || bits >= 2 * prec) return false; + /* For the time being, require operands to have the same sign. */ + if (unsignedp != TYPE_UNSIGNED (TREE_TYPE (mop2))) + return false; + machine_mode mode = TYPE_MODE (optype); optab tab = unsignedp ? umul_highpart_optab : smul_highpart_optab; if (optab_handler (tab, mode) == CODE_FOR_nothing) diff --git a/gcc/tree.def b/gcc/tree.def index 36b91f0..0aece802 100644 --- a/gcc/tree.def +++ b/gcc/tree.def @@ -700,7 +700,9 @@ DEFTREECODE (POINTER_PLUS_EXPR, "pointer_plus_expr", tcc_binary, 2) DEFTREECODE (POINTER_DIFF_EXPR, "pointer_diff_expr", tcc_binary, 2) /* Highpart multiplication. For an integral type with precision B, - returns bits [2B-1, B] of the full 2*B product. */ + returns bits [2B-1, B] of the full 2*B product. Both operands + and the result should have integer types of the same precision + and signedness. */ DEFTREECODE (MULT_HIGHPART_EXPR, "mult_highpart_expr", tcc_binary, 2) /* Division for integer result that rounds the quotient toward zero. */ @@ -1349,10 +1351,12 @@ DEFTREECODE (WIDEN_SUM_EXPR, "widen_sum_expr", tcc_binary, 2) DEFTREECODE (SAD_EXPR, "sad_expr", tcc_expression, 3) /* Widening multiplication. - The two arguments are of type t1. - The result is of type t2, such that t2 is at least twice - the size of t1. WIDEN_MULT_EXPR is equivalent to first widening (promoting) - the arguments from type t1 to type t2, and then multiplying them. */ + The two arguments are of type t1 and t2, both integral types that + have the same precision, but possibly different signedness. + The result is of integral type t3, such that t3 is at least twice + the size of t1/t2. WIDEN_MULT_EXPR is equivalent to first widening + (promoting) the arguments from type t1 to type t3, and from t2 to + type t3 and then multiplying them. */ DEFTREECODE (WIDEN_MULT_EXPR, "widen_mult_expr", tcc_binary, 2) /* Widening multiply-accumulate.