From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1034.google.com (mail-pj1-x1034.google.com [IPv6:2607:f8b0:4864:20::1034]) by sourceware.org (Postfix) with ESMTPS id F05DC3857B8E for ; Sun, 12 Jun 2022 17:16:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org F05DC3857B8E Received: by mail-pj1-x1034.google.com with SMTP id k5-20020a17090a404500b001e8875e6242so3878250pjg.5 for ; Sun, 12 Jun 2022 10:16:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:references:from:in-reply-to :content-transfer-encoding; bh=rFHCi5yBh+yAL8u5m+taSrBWYRkgywHJk3LR9WtPt74=; b=vpYPlHCrtXi4NJfXbleIi9+Pxsn0FtA7LTnOSD1FKJaeFEAdbxVgN5AQNofqsDJbfe QRnPTJNcwvzeXyUANs7AhpwTjEmnuMCEdCAsAYar6zGA6hXsOQtEzVUxigRaYjTdMVg6 B/S6VKTRnBoguCx27wXSvnC7ZilpzzbUXPjZ8XIHD7meWK14bDzOsh5cB+ZXBccKZ5AK hEz+zmdYz6rkybiMJ5Vv2WBF4yb62jwKfuB2MoxAqi4aVjvwTy0pAJP81vlBMXAG1WFo v3wc1I1R5xG8pKEdr1BhR2LbfP6VXQ1Wgf9C8tD5NinMCNhsasiOG5m4/YODVNLMWX8b 6udg== X-Gm-Message-State: AOAM531Qn5q612dTOnhWtynPcTzCq0WRcKCXXuPcP6E8I0/fWIn7C9Df +HX2egoaqF35N4kqWf0+MyIsTC5pEf0Icg== X-Google-Smtp-Source: ABdhPJzeYA1zD8CIl2Gqb4V4M/lF8wKzG8msvJpVZPocEGw0c+ff7REV9Dy9l6xCw3EAVviXCkglVg== X-Received: by 2002:a17:90b:3a8f:b0:1e8:7669:8a2f with SMTP id om15-20020a17090b3a8f00b001e876698a2fmr11196532pjb.55.1655054163764; Sun, 12 Jun 2022 10:16:03 -0700 (PDT) Received: from [172.31.0.204] (c-73-63-24-84.hsd1.ut.comcast.net. [73.63.24.84]) by smtp.gmail.com with ESMTPSA id t17-20020a170902b21100b0015e8d4eb1d5sm3324723plr.31.2022.06.12.10.16.03 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 12 Jun 2022 10:16:03 -0700 (PDT) Message-ID: Date: Sun, 12 Jun 2022 11:16:02 -0600 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.10.0 Subject: Re: [PATCH] PR middle-end/105853: Call store_constructor directly from calls.cc. Content-Language: en-US To: Roger Sayle , gcc-patches@gcc.gnu.org References: <001601d879ac$584d33e0$08e79ba0$@nextmovesoftware.com> From: Jeff Law In-Reply-To: <001601d879ac$584d33e0$08e79ba0$@nextmovesoftware.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, 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: Sun, 12 Jun 2022 17:16:07 -0000 On 6/6/2022 7:50 AM, Roger Sayle wrote: > This patch fixes both ICE regressions PR middle-end/105853 and > PR target/105856 caused by my recent patch to expand small const structs > as immediate constants. That patch updated code generation in three > places: two in expr.cc that call store_constructor directly, and the > third in calls.cc's load_register_parameters that expands its CONSTRUCTOR > via expand_expr, as store_constructor is local/static to expr.cc, and > the "public" API, should usually simply forward the constructor to the > appropriate store_constructor function. > > Alas, despite the clean regression testing on multiple targets, the above > ICEs show that expand_expr isn't a suitable proxy for store_constructor, > and things that (I'd assumed) shouldn't affect how/whether a struct is > placed in a register [such as whether the struct is considered packed/ > aligned or not] actually interfere with the optimization that is being > attempted. > > The (proposed) solution is to export store_constructor (and it's helper > function int_expr_size) from expr.cc, by removing their static qualifier > and prototyping both functions in expr.h, so they can be called directly > from load_register_parameters in calls.cc. This cures both ICEs, but > almost as important produces much better code generation than GCC 12. > > For PR 105853, GCC 12 generates: > > compose_nd_na_ipv6_src: > movzx eax, WORD PTR eth_addr_zero[rip+2] > movzx edx, WORD PTR eth_addr_zero[rip] > movzx edi, WORD PTR eth_addr_zero[rip+4] > sal rax, 16 > or rax, rdx > sal rdi, 32 > or rdi, rax > xor eax, eax > jmp packet_set_nd > eth_addr_zero: .zero 6 > > where now (with this fix) GCC 13 generates: > compose_nd_na_ipv6_src: > xorl %edi, %edi > xorl %eax, %eax > jmp packet_set_nd > > Likewise, for PR 105856 on ARM, we'd previously generate: > g_329_3: > movw r3, #:lower16:.LANCHOR0 > movt r3, #:upper16:.LANCHOR0 > ldr r0, [r3] > b func_19 > > but with this optimization we now generate: > g_329_3: > mov r0, #6 > b func_19 > > > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap > and make -k check with no new failures. I've also confirmed that on a > cross-compiler to arm-linux-gnueabihf --with-arch=armv6 this fixes the > target specific ICE in PR105856. The make check is currently running > with --target_board=unix{-m32}, OK for mainline if that also passes? > > My sincere apologies for the inconvenience. > > > 2022-06-06 Roger Sayle > > gcc/ChangeLog > PR middle-end/105853 > PR target/105856 > * calls.cc (load_register_parameters): Call store_constructor > (and int_Expr_size) directly instead of expanding via expand_expr. > * expr.cc (static void store_constructor): Don't prototype here. > (static HOST_WIDE_INT int_expr_size): Likewise. > (store_constructor): No longer static. > (int_expr_size): Likewise, no longer static. > * expr.h (store_constructor): Prototype here. > (int_expr_size): Prototype here. > > gcc/testsuite/ChangeLog > PR middle-end/105853 > PR target/105856 > * gcc.dg/pr105853.c: New test case. > * gcc.dg/pr105856.c: New test case. OK jeff