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 EB96B385782A for ; Mon, 13 Sep 2021 09:54:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org EB96B385782A 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 9B8BA6D; Mon, 13 Sep 2021 02:54:40 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.126]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 0BE473F59C; Mon, 13 Sep 2021 02:54:39 -0700 (PDT) From: Richard Sandiford To: Richard Earnshaw Mail-Followup-To: Richard Earnshaw , Richard Earnshaw via Gcc-patches , Richard Earnshaw , richard.sandiford@arm.com Cc: Richard Earnshaw via Gcc-patches , Richard Earnshaw Subject: Re: [PATCH v3 1/3] rtl: directly handle MEM in gen_highpart [PR102125] References: <20210910144841.3139174-1-rearnsha@arm.com> <20210910144841.3139174-2-rearnsha@arm.com> Date: Mon, 13 Sep 2021 10:54:38 +0100 In-Reply-To: (Richard Earnshaw's message of "Mon, 13 Sep 2021 10:51:05 +0100") 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, SPF_HELO_NONE, SPF_PASS, TXREP 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: Mon, 13 Sep 2021 09:54:42 -0000 Richard Earnshaw writes: > On 13/09/2021 10:38, Richard Sandiford via Gcc-patches wrote: >> Richard Earnshaw via Gcc-patches writes: >>> gen_lowpart_general handles forming a lowpart of a MEM by using >>> adjust_address to rework and validate a new version of the MEM. >>> Do the same for gen_highpart rather than calling simplify_gen_subreg >>> for this case. >> >> Looks OK, but what went wrong with the existing code? Did >> simplify_gen_subreg refuse to handle a MEM that you wanted >> it to handle, or did the validize_mem go wrong for some reason? > > It refused to handle it and simply returned (subreg (mem)) - see the > discussion on version 1 of the patch series. OK, that's good then. The patch is OK from my POV too FWIW. Richard >>> gcc/ChangeLog: >>> >>> PR target/102125 >>> * emit-rtl.c (gen_highpart): Use adjust_address to handle >>> MEM rather than calling simplify_gen_subreg. >>> --- >>> gcc/emit-rtl.c | 23 +++++++++++++---------- >>> 1 file changed, 13 insertions(+), 10 deletions(-) >>> >>> diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c >>> index 77ea8948ee8..0ba110879aa 100644 >>> --- a/gcc/emit-rtl.c >>> +++ b/gcc/emit-rtl.c >>> @@ -1585,19 +1585,22 @@ gen_highpart (machine_mode mode, rtx x) >>> gcc_assert (known_le (msize, (unsigned int) UNITS_PER_WORD) >>> || known_eq (msize, GET_MODE_UNIT_SIZE (GET_MODE (x)))); >>> >>> - result = simplify_gen_subreg (mode, x, GET_MODE (x), >>> - subreg_highpart_offset (mode, GET_MODE (x))); >>> - gcc_assert (result); >>> - >>> - /* simplify_gen_subreg is not guaranteed to return a valid operand for >>> - the target if we have a MEM. gen_highpart must return a valid operand, >>> - emitting code if necessary to do so. */ >>> - if (MEM_P (result)) >>> + /* gen_lowpart_common handles a lot of special cases due to needing to handle >>> + paradoxical subregs; it only calls simplify_gen_subreg when certain that >>> + it will produce something meaningful. The only case we need to handle >>> + specially here is MEM. */ >>> + if (MEM_P (x)) >>> { >>> - result = validize_mem (result); >>> - gcc_assert (result); >>> + poly_int64 offset = subreg_highpart_offset (mode, GET_MODE (x)); >>> + return adjust_address (x, mode, offset); >>> } >>> >>> + result = simplify_gen_subreg (mode, x, GET_MODE (x), >>> + subreg_highpart_offset (mode, GET_MODE (x))); >>> + /* Since we handle MEM directly above, we should never get a MEM back >>> + from simplify_gen_subreg. */ >>> + gcc_assert (result && !MEM_P (result)); >>> + >>> return result; >>> } >>>