From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from server.nextmovesoftware.com (server.nextmovesoftware.com [162.254.253.69]) by sourceware.org (Postfix) with ESMTPS id 56B373874766; Wed, 8 Jun 2022 13:06:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 56B373874766 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=F0skSmmRRLaxqP0cykmAkZxGFVyBidobncX44Uk8AbA=; b=Q0vyuQYMXBj4ESlTolJHlYlbBS W3dN+RMdGAk8sbdaIb7sXKpZ34j0S/r48EgQK4sdljLr5WDDcJYYno+8PfWUHHU/6E9dApi6q392Y Gyoi6qzqNCUai0bpC3pj1TZoBy9Aw0iS1Fy59SF5ywBuKDOyZgpEmdSSJ/S24hp7xjzeYBhCSE7TO VoSD4Y4bZDBmauScJOk0W9Ywo/Xxi7r5KUPc1gzsBCgxs4s38F2X4VYWVguWongZHGK8ge4eWtEZO 2ZXWhowXMdRC6QysG9PlDH0FSvd7Sm8cgm0Ot0wlm97Ahz5gEi2+KviJIfRynrAPWGFLXok/gJ/wr yDFRZD0Q==; Received: from [185.62.158.67] (port=59152 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 1nyvOA-0006Sg-Oy; Wed, 08 Jun 2022 09:06:22 -0400 From: "Roger Sayle" To: Cc: "'Eric Botcazou'" , "'Jeff Law'" , "'Tamar Christina'" Subject: [PATCH] PR middle-end/105874: Use EXPAND_MEMORY to fix ada bootstrap. Date: Wed, 8 Jun 2022 14:06:19 +0100 Message-ID: <07f101d87b38$8c2b42a0$a481c7e0$@nextmovesoftware.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_NextPart_000_07F2_01D87B40.EDEFAAA0" X-Mailer: Microsoft Outlook 16.0 Thread-Index: Adh7N7q4GQP7zpJHQqapVYWqFSWlZA== 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.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE 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: , X-List-Received-Date: Wed, 08 Jun 2022 13:06:33 -0000 This is a multipart message in MIME format. ------=_NextPart_000_07F2_01D87B40.EDEFAAA0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Many thanks to Tamar Christina for filing PR middle-end/105874 indicating that SPECcpu 2017's Leela is failing on x86_64 due to a miscompilation of FastBoard::is_eye. This function is much smaller and easier to work with than my previous hunt for the cause of the Ada bootstrap failures due to miscompilation somewhere in GCC (or one of the 131 places that the problematic form of optimization triggers during an ada bootstrap). It turns out the source of the miscompilation introduced by my recent patch is the distinction (during RTL expansion) of l-values and r-values. According to the documentation above expand_modifier, EXPAND_MEMORY should be used for lvalues (when a memory is required), and EXPAND_NORMAL for rvalues when a constant is permissible. In what I'd like to consider a latent bug, the recursive call to expand_expr_real on line 11188 of expr.cc, in the case handling ARRAY_REF, COMPONENT_REF, BIT_FIELD_REF and ARRARY_RANGE_REF was passing EXPAND_NORMAL when it really required (the semantics of) EXPAND_MEMORY. All the time that VAR_DECLs were being returned as memory this was fine, but as soon as we're able to optimize sort arrays into immediate constants, bad things happen. In the test case from Leela, we notice that the array s_eyemask always has DImode constant value { 4, 64 }, which is useful as an rvalue, but not when we need to index it as an lvalue, as in s_eyemask[color]. This also explains why everything being accepted by immediate_const_ctor_p (during an ada bootstrap) looks reasonable, what's incorrect is that we don't know how these structs/arrays are to be used. The fix is to ensure that we call expand_expr with EXPAND_MEMORY when processing the VAR_DECL's returned by get_inner_reference. This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and make -k check (with no new failures), but also with --enable-languages="ada" where it allows the bootstrap to finish, and with no unexpected failures in the acats and gnat testsuites. Ok for mainline? 2022-06-08 Roger Sayle gcc/ChangeLog PR middle-end/105874 * gcc/expr.cc (expand_expr_real_1) : New local variable tem_modifier for calculating the expand_modifier enum to use for expanding tem. If tem is a VAR_DECL, use EXPAND_MEMORY. gcc/testsuite/ChangeLog PR middle-end/105874 * g++.dg/opt/pr105874.C: New test case. Sorry again for the inconvenience/breakage. Roger -- ------=_NextPart_000_07F2_01D87B40.EDEFAAA0 Content-Type: text/plain; name="patchem.txt" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="patchem.txt" diff --git a/gcc/expr.cc b/gcc/expr.cc=0A= index fb062dc..a013650 100644=0A= --- a/gcc/expr.cc=0A= +++ b/gcc/expr.cc=0A= @@ -11183,6 +11183,13 @@ expand_expr_real_1 (tree exp, rtx target, = machine_mode tmode,=0A= infinitely recurse. */=0A= gcc_assert (tem !=3D exp);=0A= =0A= + /* If tem is a VAR_DECL, we need a memory reference. */=0A= + enum expand_modifier tem_modifier =3D modifier;=0A= + if (tem_modifier =3D=3D EXPAND_SUM)=0A= + tem_modifier =3D EXPAND_NORMAL;=0A= + if (TREE_CODE (tem) =3D=3D VAR_DECL)=0A= + tem_modifier =3D EXPAND_MEMORY;=0A= +=0A= /* If TEM's type is a union of variable size, pass TARGET to the inner=0A= computation, since it will need a temporary and TARGET is known=0A= to have to do. This occurs in unchecked conversion in Ada. */=0A= @@ -11194,9 +11201,7 @@ expand_expr_real_1 (tree exp, rtx target, = machine_mode tmode,=0A= !=3D INTEGER_CST)=0A= && modifier !=3D EXPAND_STACK_PARM=0A= ? target : NULL_RTX),=0A= - VOIDmode,=0A= - modifier =3D=3D EXPAND_SUM ? EXPAND_NORMAL : modifier,=0A= - NULL, true);=0A= + VOIDmode, tem_modifier, NULL, true);=0A= =0A= /* If the field has a mode, we want to access it in the=0A= field's mode, not the computed mode.=0A= diff --git a/gcc/testsuite/g++.dg/opt/pr105874.C = b/gcc/testsuite/g++.dg/opt/pr105874.C=0A= new file mode 100644=0A= index 0000000..58699a6=0A= --- /dev/null=0A= +++ b/gcc/testsuite/g++.dg/opt/pr105874.C=0A= @@ -0,0 +1,30 @@=0A= +/* { dg-do compile } */=0A= +/* { dg-options "-O2 -std=3Dc++11" } */=0A= +#include =0A= +=0A= +static constexpr int NBR_SHIFT =3D 4;=0A= +=0A= +static constexpr int MAXBOARDSIZE =3D 25;=0A= +=0A= +static constexpr int MAXSQ =3D ((MAXBOARDSIZE + 2) * (MAXBOARDSIZE + = 2));=0A= +=0A= +enum square_t : char {=0A= + BLACK =3D 0, WHITE =3D 1, EMPTY =3D 2, INVAL =3D 3=0A= + };=0A= +=0A= +const std::array s_eyemask =3D {=0A= + 4 * (1 << (NBR_SHIFT * BLACK)),=0A= + 4 * (1 << (NBR_SHIFT * WHITE))=0A= +};=0A= +=0A= +/* counts of neighboring stones */=0A= +std::array m_neighbours;=0A= +=0A= +int is_eye(const int color, const int i) {=0A= + /* check for 4 neighbors of the same color */=0A= + int ownsurrounded =3D (m_neighbours[i] & s_eyemask[color]);=0A= +=0A= + return ownsurrounded;=0A= +}=0A= +=0A= +/* { dg-final { scan-assembler "s_eyemask" } } */=0A= ------=_NextPart_000_07F2_01D87B40.EDEFAAA0--