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 B7B6D385802D for ; Mon, 2 Nov 2020 19:03:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org B7B6D385802D 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 49444139F; Mon, 2 Nov 2020 11:03:44 -0800 (PST) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.126]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 8894F3F719; Mon, 2 Nov 2020 11:03:43 -0800 (PST) From: Richard Sandiford To: Hongtao Liu Mail-Followup-To: Hongtao Liu , Hongtao Liu via Gcc-patches , Vladimir Makarov , dcb314@hotmail.com, richard.sandiford@arm.com Cc: Hongtao Liu via Gcc-patches , Vladimir Makarov , dcb314@hotmail.com Subject: Re: [PATCH][PR target/97540] Don't extract memory from operand for normal memory constraint. References: Date: Mon, 02 Nov 2020 19:03:42 +0000 In-Reply-To: (Hongtao Liu's message of "Mon, 2 Nov 2020 15:12:32 +0800") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-6.6 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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, 02 Nov 2020 19:03:46 -0000 Hongtao Liu writes: > On Fri, Oct 30, 2020 at 1:00 AM Richard Sandiford > wrote: >> >> I guess my main objection is that we have a special memory constraint >> that isn't in fact matching a MEM (at least not directly). That seems >> odd and feels like it's going to come back to bite us. >> >> From an RTL perspective, the MEM in the bcst_memory_operand isn't that >> special. It's really just a normal memory that happens to appear in a >> VEC_DUPLICATE. >> >> With the MIPS thing, the SIGN_EXTEND was around a register rather >> than a memory, and the register constraints applied to the register >> as normal. In other words, the SIGN_EXTEND was not part of the >> constraint: target-independent code removed the SIGN_EXTEND > > Yes, that's because target-independent code can't distinguish whether > SIGN_EXTEND is part of constraints. More specifically, constrain_operands > will swallow the unary operator when matching constraints. Cut from reco= g.c > ----- > /* A unary operator may be accepted by the predicate, but it > is irrelevant for matching constraints. */ > /* For special_memory_operand, there could be a memory operand = inside, > and it would cause a mismatch for constraint_satisfied_p. */ > if (UNARY_P (op) && op =3D=3D extract_mem_from_operand (op)) > op =3D XEXP (op, 0); > ------ Yeah, this is a vestige from the old code to support MIPS-style SIGN_EXTEND constraints. I can't remember why it wasn't removed. Maybe noone thought about it, or maybe it's what H-P said about SH. reload also still has code for this, but it's telling that LRA and IRA don'= t. > But a unary operator with memory would never be accepted by the predicate > unless it's corresponding to special_memory_constraint, because in IRA/LR= A, > CT_MEMORY would never be matched when op is false for MEM_P. I think my point is kind of the opposite though: in both the MIPS and the AVX512 cases, we effectively want the constrained operand to be different from the operand matched by the predicate. So I think the old way of getting target-independent code to dig down into unary expressions is conceptually cleaner. Even better would be to have .md constructs that say exactly what the constraints should match for a given predicate, but that's obviously more work (and probably not for GCC 11). I realise it's a bit odd to be warning on the one hand that the old approach seemed to have problems while at the same time advocating for following something like the old approach, but still. :-) For example, there's no conceptual reason why a target couldn't support VEC_DUPLICATEs of registers. And if it supported VEC_DUPLICATEs of both registers and memory, we'd want to allow registers to be spilled to memory in-place, without generating a reload. To do that, the thing being constrained has to be the operand of the VEC_DUPLICATE rather than the VEC_DUPLICATE itself. So for AVX512 I think we should handle this by getting target-independent code to look into the VEC_DUPLICATE and make =E2=80=9CBr=E2=80=9D match a n= ormal (scalar) memory operand whose mode satisfies VALID_BCST_MODE_P. I'm experimenting with introducing accessors for getting the =E2=80=9Cconstraint version=E2=80=9D of recog_data.operand, recog_data.oper= and_loc and recog_data.operand_mode (hopefully low-overhead). For now they just look inside UNARY_P, as before. Too early to say whether it'll work though=E2=80=A6 Thanks, Richard