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 031A83835E1D for ; Tue, 6 Dec 2022 09:36:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 031A83835E1D Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com 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 492F023A; Tue, 6 Dec 2022 01:36:32 -0800 (PST) Received: from localhost (e121540-lin.manchester.arm.com [10.32.99.50]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 84CF13F73B; Tue, 6 Dec 2022 01:36:24 -0800 (PST) From: Richard Sandiford To: Richard Biener via Gcc-patches Mail-Followup-To: Richard Biener via Gcc-patches ,HAO CHEN GUI , Richard Biener , Segher Boessenkool , David , "Kewen.Lin" , Peter Bergner , richard.sandiford@arm.com Cc: HAO CHEN GUI , Richard Biener , Segher Boessenkool , David , "Kewen.Lin" , Peter Bergner Subject: Re: [PATCH] Add a new conversion for conditional ternary set into ifcvt [PR106536] References: <75fb4899-ceb2-e6a9-0dd4-577de9a8b976@linux.ibm.com> <937d6047-e6e0-cbc9-2ed1-13a221f5a361@linux.ibm.com> Date: Tue, 06 Dec 2022 09:36:23 +0000 In-Reply-To: (Richard Biener via Gcc-patches's message of "Thu, 24 Nov 2022 11:24:15 +0100") 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=-32.9 required=5.0 tests=BAYES_00,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Richard Biener via Gcc-patches writes: > On Thu, Nov 24, 2022 at 8:25 AM HAO CHEN GUI wrot= e: >> >> Hi Richard, >> >> >> =E5=9C=A8 2022/11/24 4:06, Richard Biener =E5=86=99=E9=81=93: >> > Wouldn't we usually either add an optab or try to recog a canonical >> > RTL form instead of adding a new target hook for things like this? >> >> Thanks so much for your comments. Please let me make it clear. >> >> Do you mean we should create an optab for "setb" pattern (the nested >> if-then-else insn) and detect candidate insns in ifcvt pass? Then >> generate the insn with the new optab? > > Yes, that would be one way to do it. Another way would be to > generate a (to be defined) canonical form of such instruction and > see whether it can be recognized (whether there's a define_insn > for it). > > Note that were just things that came into my mind here, I'm not too > familiar with how we handle such situations but at least I'm not > aware of dozens of target hooks to handle instruction availability. Yeah, this was my reaction too. The patch does use recog for the conditional set itself, which is good, but I'm not sure from the patch what the target hook is supposed to do in preparation for the recog. I think it'd be better to avoid having too many hooks that take noce_if_info parameters. It's really just a bunch of internal pass state, rather than something that was designed to be a public interface. Currently we have one hook that takes noce_if_info parameters, for costing. That's still one more than I'd like, but at least there are no correctness concerns and, if someone changes noce_if_info in future, just rebuilding cc1 for the affected targets should be good enough as far as testing goes. If we start using hooks for code generation too, we're effectively distributing the ifcvt pass across targets, which makes the pass harder to maintain. Thanks, Richard