From patchwork Fri Mar 4 10:30:19 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roger Sayle X-Patchwork-Id: 1600937 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=EO5Mpwxx; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4K93zL2Dl5z9sG9 for ; Fri, 4 Mar 2022 21:30:52 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 28E6F3858417 for ; Fri, 4 Mar 2022 10:30:47 +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 16DFD3858D39 for ; Fri, 4 Mar 2022 10:30:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 16DFD3858D39 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=AKtg5cqBBie0gveN7OwliJJriUh94AJBA6MVSUWP4K0=; b=EO5Mpwxx0Q6jpGOWoZhPlrDOGo 6VRj/0I/nsP+8AJYlQVhHLrQv0c0qRO4iQwYuNoKQ4zwTNvjVlcWGuXrJr7hfGOG6fTceODDeEPNF 2kIgl8gZKZESb9siku3eA+aR8BlwIqX70Q1wsY4TaF8sjpYj8Iyky5Wcgj/fprYNXrtuo9pe76Kr2 pcT2fjyDBpUPNCUhc2mDkBTECL0S3VQoUyiU69NuQhYkOyGv37R6TaL4I2AW/CkTGpsAcVWXEyThS mkWKh8PUATruB3HGLwie34kKmDEcgyD7wdH6k6eZflmopiNOY920p8/HZ/BTH+Mjr5bpwtMRQR6TX YB30+T8A==; Received: from host86-186-213-42.range86-186.btcentralplus.com ([86.186.213.42]:52822 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 1nQ5CY-0005qI-9i; Fri, 04 Mar 2022 05:30:22 -0500 From: "Roger Sayle" To: Subject: [i386 PATCH] PR 104732: Simplify/fix DI mode logic expansion/splitting on -m32. Date: Fri, 4 Mar 2022 10:30:19 -0000 Message-ID: <004501d82fb2$d9e77b20$8db67160$@nextmovesoftware.com> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AdgvsXmcmy49Q1blQpy+Yu8bNv6MJw== 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.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE 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-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 clean-up patch resolves PR testsuite/104732, the failure of the recent test gcc.target/i386/pr100711-1.c on 32-bit Solaris/x86. Rather than just tweak the testcase, the proposed approach is to fix the underlying problem by removing the "TARGET_STV && TARGET_SSE2" conditionals from the DI mode logical operation expanders and pre-reload splitters in i386.md, which as I'll show generate inferior code (even a GCC 12 regression) on !TARGET_64BIT whenever -mno-stv (such as Solaris) or -msse (but not -msse2). First a little bit of history. In the beginning, DImode operations on i386 weren't defined by the machine description, and lowered during RTL expansion to SI mode operations. The with PR 65105 in 2015, -mstv was added, together with a SWIM1248x mode iterator (later renamed to SWIM1248x) together with several *di3_doubleword post-reload splitters that made use of register allocation to perform some double word operations in 64-but XMM registers. A short while later in 2016, PR 70322 added similar support for one_cmpldi2. All of this logic was dependent upon "!TARGET_64BIT && TARGET_STV && TARGET_SSE2". With the passing of time, these conditions became irrelevant when in 2019, it was decided to split these double-word patterns before reload. https://gcc.gnu.org/pipermail/gcc-patches/2019-June/523877.html https://gcc.gnu.org/pipermail/gcc-patches/2019-October/532236.html Hence the current situation, where on most modern CPU architectures (where "TARGET_STV && TARGET_SSE2" is true), RTL is expanded with DI mode operations, that are then split into two SI mode instructions before reload, except on Solaris and other odd cases, where the splitting is to two SI mode instructions is done during RTL expansion. By the time compilation reaches register allocation both paths in theory produce identical or similar code, so the vestigial legacy/logic would appear to be harmless. Unfortunately, there is one place where this arbitrary choice of how to lower DI mode doubleword operations is visible to the middle-end, it controls whether the backend appears to have a suitable optab, and the presence (or not) of DImode optabs can influence vectorization cost models and veclower decisions. The issue (and code quality regression) can be seen in this test case: typedef long long v2di __attribute__((vector_size (16))); v2di x; void foo (long long a) { v2di t = {a, a}; x = ~t; } which when compiled with "-O2 -m32 -msse -march=pentiumpro" produces: foo: subl $28, %esp movl %ebx, 16(%esp) movl 32(%esp), %eax movl %esi, 20(%esp) movl 36(%esp), %edx movl %edi, 24(%esp) movl %eax, %esi movl %eax, %edi movl %edx, %ebx movl %edx, %ecx notl %esi notl %ebx movl %esi, (%esp) notl %edi notl %ecx movl %ebx, 4(%esp) movl 20(%esp), %esi movl %edi, 8(%esp) movl 16(%esp), %ebx movl %ecx, 12(%esp) movl 24(%esp), %edi movss 8(%esp), %xmm1 movss 12(%esp), %xmm2 movss (%esp), %xmm0 movss 4(%esp), %xmm3 unpcklps %xmm2, %xmm1 unpcklps %xmm3, %xmm0 movlhps %xmm1, %xmm0 movaps %xmm0, x addl $28, %esp ret Importantly notice the four "notl" instructions. With this patch: foo: subl $28, %esp movl 32(%esp), %edx movl 36(%esp), %eax notl %edx movl %edx, (%esp) notl %eax movl %eax, 4(%esp) movl %edx, 8(%esp) movl %eax, 12(%esp) movaps (%esp), %xmm1 movaps %xmm1, x addl $28, %esp ret Notice only two "notl" instructions. Checking with Godbolt.org, GCC generated 4 NOTs in GCC 4.x and 5.x, 2 NOTs between GCC 6.x and 9.x, and regressed to 4 NOTs since GCC 10.x [which hopefully qualifies this clean-up as suitable for stage 4]. Most significantly, this patch allows pr100711-1.c to pass with -mno-stv, allowing pandn to be used with V2DImode on Solaris/x86. Fingers-crossed this should reduce the number of discrepancies that Rainer Orth encounters supporting Solaris/x86. This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and make -k check, with/without --target_board='unix{-m32\ -march= cascadelake}' with no new failures. Ok for mainline? 2022-03-04 Roger Sayle gcc/ChangeLog PR testsuite/104732 * config/i386/i386.md (SWIM1248s): Include DI mode unconditionally. (*anddi3_doubleword): Remove && TARGET_STV && TARGET_SSE2 condition, i.e. always split on !TARGET_64BIT. (*di3_doubleword): Likewise. (*one_cmpldi2_doubleword): Likewise. gcc/testsuite/ChangeLog PR testsuite/104732 * gcc.target/i386/pr104732.c: New test case. Thanks in advance, Roger diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index 5e0a980..8eae4b0 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -1079,11 +1079,11 @@ (HI "TARGET_HIMODE_MATH") SI]) -;; Math-dependant integer modes with DImode (enabled for 32bit with STV). +;; Math-dependant integer modes with DImode. (define_mode_iterator SWIM1248s [(QI "TARGET_QIMODE_MATH") (HI "TARGET_HIMODE_MATH") - SI (DI "TARGET_64BIT || (TARGET_STV && TARGET_SSE2)")]) + SI DI]) ;; Math-dependant single word integer modes without QImode. (define_mode_iterator SWIM248 [(HI "TARGET_HIMODE_MATH") @@ -9733,7 +9733,7 @@ (match_operand:DI 1 "nonimmediate_operand") (match_operand:DI 2 "x86_64_szext_general_operand"))) (clobber (reg:CC FLAGS_REG))] - "!TARGET_64BIT && TARGET_STV && TARGET_SSE2 + "!TARGET_64BIT && ix86_binary_operator_ok (AND, DImode, operands) && ix86_pre_reload_split ()" "#" @@ -10349,7 +10349,7 @@ (match_operand:DI 1 "nonimmediate_operand") (match_operand:DI 2 "x86_64_szext_general_operand"))) (clobber (reg:CC FLAGS_REG))] - "!TARGET_64BIT && TARGET_STV && TARGET_SSE2 + "!TARGET_64BIT && ix86_binary_operator_ok (, DImode, operands) && ix86_pre_reload_split ()" "#" @@ -11435,7 +11435,7 @@ (define_insn_and_split "*one_cmpldi2_doubleword" [(set (match_operand:DI 0 "nonimmediate_operand") (not:DI (match_operand:DI 1 "nonimmediate_operand")))] - "!TARGET_64BIT && TARGET_STV && TARGET_SSE2 + "!TARGET_64BIT && ix86_unary_operator_ok (NOT, DImode, operands) && ix86_pre_reload_split ()" "#" diff --git a/gcc/testsuite/gcc.target/i386/pr104732.c b/gcc/testsuite/gcc.target/i386/pr104732.c new file mode 100644 index 0000000..c8954366c --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr104732.c @@ -0,0 +1,14 @@ +/* { dg-do compile { target ia32 } } */ +/* { dg-options "-O2 -m32 -msse -march=pentiumpro" } */ + +typedef long long v2di __attribute__((vector_size (16))); + +v2di x; + +void foo (long long a) +{ + v2di t = {a, a}; + x = ~t; +} + +/* { dg-final { scan-assembler-times "notl" 2 } } */