From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate.crashing.org (gate.crashing.org [63.228.1.57]) by sourceware.org (Postfix) with ESMTP id BBFC23858C66 for ; Mon, 6 Mar 2023 23:32:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BBFC23858C66 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=kernel.crashing.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=kernel.crashing.org Received: from gate.crashing.org (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.14.1) with ESMTP id 326NVhst028038; Mon, 6 Mar 2023 17:31:43 -0600 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 326NVghc028037; Mon, 6 Mar 2023 17:31:42 -0600 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Mon, 6 Mar 2023 17:31:42 -0600 From: Segher Boessenkool To: Jakub Jelinek , Jeff Law via Gcc-patches , Tamar Christina , Roger Sayle , Jeff Law , richard.sandiford@arm.com Subject: Re: [PATCH] combine: Try harder to form zero_extends [PR106594] Message-ID: <20230306233142.GR25951@gate.crashing.org> References: <20230304221749.GK25951@gate.crashing.org> <3b1ed616-5d90-7a66-63b5-bdb5e320eebf@gmail.com> <20230306135850.GN25951@gate.crashing.org> <20230306183151.GP25951@gate.crashing.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,JMQ_SPF_NEUTRAL,KAM_DMARC_STATUS,KAM_MANYTO,SPF_HELO_PASS,SPF_PASS,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: Hi! On Mon, Mar 06, 2023 at 07:13:08PM +0000, Richard Sandiford wrote: > Segher Boessenkool writes: > > Most importantly, what makes you think this is a problem for aarch64 > > only? If it actually is, you can fix it in the aarch64 config! Either > > with or without new hooks, whatever works best. > > The point is that I don't think it's a problem for AArch64 only. > I think it's a generic issue that should be solved in a generic way > (which is what the patch is trying to do). The suggestion to restrict > it to AArch64 came from Jakub. > > The reason I'm pushing back against a hook is precisely because > I don't want to solve this in AArch64-specific code. But it is many times worse still to do it in target-specific magic code disguised as generic code :-( If there is no clear explanation why combine should do X, then it probably should not. > I'm not sure we would be talking about restricting this to AArch64 > if the patch had been posted in stage 1. If people are concerned > about doing this for all targets in stage 4 (which they seem to be), Not me, not in principle. But it takes more time than we have left in stage 4 to handle this, even for only combine. We should give the other target maintainers much longer as well. > I thought the #ifdef was the simplest way of addressing that concern. An #ifdef is a way of making a change that is not finished yet not hurt the other targets. It still hurts generic development, which indirectly hurts all targets. > And I don't think what the patch does is ad hoc. It is almost impossible to explain what it does and why that is a good thing, why it is what we want, what we should do here; and certainly not in a compact, terse, focused way. It has all the hallmarks of ad hoc patches. > Reorganising the > expression in this way isn't something new. extract_left_shift already > does a similar thing (and does it for all targets). That is not similar at all, no. /* See if X (of mode MODE) contains an ASHIFT of COUNT or more bits that can be commuted with any other operations in X. Return X without that shift if so. */ If you can factor out a utility function like that, with an actual nice description like that, it would be a much more palatable patch. Segher