From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id E8BE63858D3C for ; Thu, 5 May 2022 07:59:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E8BE63858D3C Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 3E2941042; Thu, 5 May 2022 00:59:23 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.37]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 79FAD3FA27; Thu, 5 May 2022 00:59:22 -0700 (PDT) From: Richard Sandiford To: Alexandre Oliva via Gcc-patches Mail-Followup-To: Alexandre Oliva via Gcc-patches , Alexandre Oliva , dje.gcc@gmail.com, segher@kernel.crashing.org, richard.sandiford@arm.com Cc: Alexandre Oliva , dje.gcc@gmail.com, segher@kernel.crashing.org Subject: Re: [PATCH] [PR100106] Reject unaligned subregs when strict alignment is required References: Date: Thu, 05 May 2022 08:59:21 +0100 In-Reply-To: (Alexandre Oliva via Gcc-patches's message of "Thu, 05 May 2022 03:52:01 -0300") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain 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: Thu, 05 May 2022 07:59:26 -0000 Alexandre Oliva via Gcc-patches writes: > 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. > > > At the time of posting this patch, it occurred to me that maybe the test > should allow paradoxical subregs of mems, or even that non-paradoxical > subregs of mems should be allowed to change to a mode with stricter > alignment, and the register allocator should deal with that somehow. > WDYT? > > > Regstrapped on x86_64-linux-gnu and ppc64le-linux-gnu, also tested > targeting ppc- and ppc64-vx7r2. Ok to install? > > > for gcc/ChangeLog > > PR target/100106 > * emit-rtl.c (validate_subreg): Reject a SUBREG of a MEM that > requires stricter alignment than MEM's. I know this is the best being the enemy of the good, but given that we're at the start of stage 1, would it be feasible to try to get rid of (subreg (mem)) altogether for GCC 13? We could do it target-by-target, with a target macro (yes, macro :-)) that opts in to keeping the existing behaviour. (subreg (mem)) would then be unconditionally invalid when the macro isn't defined. (Even in debug expressions, since those ought to narrow to a mem anyway.) Thanks, Richard > for gcc/testsuite/ChangeLog > > PR target/100106 > * gcc.target/powerpc/pr100106-sa.c: New. > --- > gcc/emit-rtl.cc | 3 +++ > gcc/testsuite/gcc.target/powerpc/pr100106-sa.c | 4 ++++ > 2 files changed, 7 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..642e47eada0d7 100644 > --- a/gcc/emit-rtl.cc > +++ b/gcc/emit-rtl.cc > @@ -982,6 +982,9 @@ validate_subreg (machine_mode omode, machine_mode imode, > > return subreg_offset_representable_p (regno, imode, offset, omode); > } > + 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..6cc29595c8b25 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/pr100106-sa.c > @@ -0,0 +1,4 @@ > +/* { dg-do compile { target { ilp32 } } } */ > +/* { dg-options "-mcpu=604 -O -mstrict-align" } */ > + > +#include "../../gcc.c-torture/compile/pr100106.c"