From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from rock.gnat.com (rock.gnat.com [205.232.38.15]) by sourceware.org (Postfix) with ESMTPS id C12DF3858407 for ; Fri, 6 May 2022 02:41:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C12DF3858407 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 8252A116776; Thu, 5 May 2022 22:41:55 -0400 (EDT) X-Virus-Scanned: Debian amavisd-new at gnat.com Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id A2KuE4JAlNDq; Thu, 5 May 2022 22:41:55 -0400 (EDT) Received: from free.home (tron.gnat.com [IPv6:2620:20:4000:0:46a8:42ff:fe0e:e294]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by rock.gnat.com (Postfix) with ESMTPS id E127911676E; Thu, 5 May 2022 22:41:54 -0400 (EDT) Received: from livre (livre.home [172.31.160.2]) by free.home (8.15.2/8.15.2) with ESMTPS id 2462fgTY2758772 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 5 May 2022 23:41:43 -0300 From: Alexandre Oliva To: Segher Boessenkool Cc: gcc-patches@gcc.gnu.org, ebotcazou@libertysurf.fr, vmakarov@redhat.com, dje.gcc@gmail.com Subject: Re: [PATCH v2] [PR100106] Reject unaligned subregs when strict alignment is required Organization: Free thinker, does not speak for AdaCore References: <20220505143308.GC25951@gate.crashing.org> Errors-To: aoliva@lxoliva.fsfla.org Date: Thu, 05 May 2022 23:41:42 -0300 In-Reply-To: <20220505143308.GC25951@gate.crashing.org> (Segher Boessenkool's message of "Thu, 5 May 2022 09:33:08 -0500") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Scanned-By: MIMEDefang 2.84 X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, 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: , X-List-Received-Date: Fri, 06 May 2022 02:41:57 -0000 On May 5, 2022, Segher Boessenkool wrote: > On Thu, May 05, 2022 at 03:52:01AM -0300, Alexandre Oliva wrote: >> + else if (reg && MEM_P (reg) >> + && STRICT_ALIGNMENT && MEM_ALIGN (reg) < GET_MODE_ALIGNMENT (omode)) >> + return false; > Please fix the line breaks? Either do a break before every &&, or put > as many things as possible on one line? I was going for conceptual grouping of alignment-related subexprs, but I don't care enough to fight for it. > Note that you should never have paradoxical subregs of mem on rs6000 or > any other target with INSN_SCHEDULING. Great, that alleviates some of my concerns about overreaching in this patch. >> +#include "../../gcc.c-torture/compile/pr100106.c" > It is better to copy the 11 lines of code. 'k > Please comment what the ilp32 is for (namely, the -mcpu= will barf > without it).. Ack > The testcase is okay with those changes, thanks! Thanks. Here's the revised patch. I'm now testing on several platforms a follow-up patch that introduces TARGET_ALLOW_SUBREG_OF_MEM. [PR100106] Reject unaligned subregs when strict alignment is required From: Alexandre Oliva The testcase for pr100106, compiled with optimization for 32-bit powerpc -mcpu=604 with -mstrict-align expands the initialization of a union from a float _Complex value into a load from an SCmode constant pool entry, aligned to 4 bytes, into a DImode pseudo, requiring 8-byte alignment. The patch that introduced the testcase modified simplify_subreg to avoid changing the MEM to outermode, but simplify_gen_subreg still creates a SUBREG or a MEM that would require stricter alignment than MEM's, and lra_constraints appears to get confused by that, repeatedly creating unsatisfiable reloads for the SUBREG until it exceeds the insn count. Avoiding the unaligned SUBREG, expand splits the DImode dest into SUBREGs and loads each SImode word of the constant pool with the proper alignment. for gcc/ChangeLog PR target/100106 * emit-rtl.cc (validate_subreg): Reject a SUBREG of a MEM that requires stricter alignment than MEM's. for gcc/testsuite/ChangeLog PR target/100106 * gcc.target/powerpc/pr100106-sa.c: New. --- gcc/emit-rtl.cc | 4 ++++ gcc/testsuite/gcc.target/powerpc/pr100106-sa.c | 15 +++++++++++++++ 2 files changed, 19 insertions(+) create mode 100644 gcc/testsuite/gcc.target/powerpc/pr100106-sa.c diff --git a/gcc/emit-rtl.cc b/gcc/emit-rtl.cc index 1e02ae254d012..9c03e27894fff 100644 --- a/gcc/emit-rtl.cc +++ b/gcc/emit-rtl.cc @@ -982,6 +982,10 @@ validate_subreg (machine_mode omode, machine_mode imode, return subreg_offset_representable_p (regno, imode, offset, omode); } + /* Do not allow SUBREG with stricter alignment than the inner MEM. */ + else if (reg && MEM_P (reg) && STRICT_ALIGNMENT + && MEM_ALIGN (reg) < GET_MODE_ALIGNMENT (omode)) + return false; /* The outer size must be ordered wrt the register size, otherwise we wouldn't know at compile time how many registers the outer diff --git a/gcc/testsuite/gcc.target/powerpc/pr100106-sa.c b/gcc/testsuite/gcc.target/powerpc/pr100106-sa.c new file mode 100644 index 0000000000000..87634efa8d0b7 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/pr100106-sa.c @@ -0,0 +1,15 @@ +/* Require ilp32 because -mcpu=604 won't do 64 bits. */ +/* { dg-do compile { target { ilp32 } } } */ +/* { dg-options "-mcpu=604 -O -mstrict-align" } */ + +union a { + float _Complex b; + long long c; +}; + +void g(union a); + +void e() { + union a f = {1.0f}; + g(f); +} -- Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer Disinformation flourishes because many people care deeply about injustice but very few check the facts. Ask me about