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 901763858414 for ; Wed, 8 Sep 2021 19:17:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 901763858414 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=kernel.crashing.org Authentication-Results: sourceware.org; spf=fail 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 188JG3hO017543; Wed, 8 Sep 2021 14:16:03 -0500 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 188JG2fd017538; Wed, 8 Sep 2021 14:16:02 -0500 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Wed, 8 Sep 2021 14:16:02 -0500 From: Segher Boessenkool To: Richard Biener Cc: Michael Meissner , GCC Patches , David Edelsohn , Bill Schmidt , Peter Bergner , Will Schmidt Subject: Re: [PATCH] Fix SFmode subreg of DImode and TImode Message-ID: <20210908191602.GQ1583@gate.crashing.org> References: <20210907230730.GM1583@gate.crashing.org> <20210908170809.GP1583@gate.crashing.org> <5DBC1101-9DD6-48F8-BC25-F4DD354B4D74@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5DBC1101-9DD6-48F8-BC25-F4DD354B4D74@gmail.com> User-Agent: Mutt/1.4.2.3i X-Spam-Status: No, score=-5.6 required=5.0 tests=BAYES_00, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, TXREP, T_SPF_HELO_PERMERROR, T_SPF_PERMERROR autolearn=no 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: Wed, 08 Sep 2021 19:17:06 -0000 On Wed, Sep 08, 2021 at 08:39:31PM +0200, Richard Biener wrote: > On September 8, 2021 7:08:09 PM GMT+02:00, Segher Boessenkool wrote: > >It is not a good idea to do allow all those things. Most backends can > >only support a few combinations of them, and everything else results in > >*worse* machine code, best case, and more and more complicated (and more > >buggy!) backend code. > > > >But that is a code quality issue. The current problem is that we have > >at least PR102211 and PR102154 (as well as reports elsewhere of bugs on > >other targets). Code that used before doesn't anymore, and we have no > >clear way out, no recommendation how to fix this and a) keep the same > >functionality without huge changes, and b) keep the same machine code > >quality. > > > >I do not think something like that can be done. That is why I am asking > >for the patch to be reverted until all of the groundwork for it has been > >done. That includes making generic testcases that show how such subregs > >behave, so that we can see in testresults what changes do to existing > >targets. > > Heh, I understood your earlier reply that you supported the change in principle based on the fact that nested subregs are invalid. Ah. No. I meant to lament the fact that we use subregs for multiple things, so that doing a bit_cast of a real subreg has to be expressed as just one subreg, which is clearly sub-optimal for most backends. I say *has to* because a subreg of a subreg is not valid RTL; it has to be written as just one subreg. Which makes thing more confusing and confused than this already non-trivial thing has to be. > Now, I don't think that validate_subreg is supposed to be the decision maker on what a target allows. Right, some target hook or macro or whatnot should. > For subregs of hardregs we seem to have a good way of validating, but > what do we have for subregs of pseudos? Is it the passes generating > the new unsupported subregs that should do different things? Should > validate_subreg use a target hook to allow those special casings we > removed which all were necessary just for specific targets but > appearantly did not do any harm for other targets? Currently, we can disallow things in predicates and/or the instruction conditions. > Can you give advice as to how to address the needs of the HFmode subregs on x86 if not by adding another (generic) narrow exception in validate_subreg? If I knew what the problem was, perhaps? Is this explained in some mail somewhere? > That said, I fail to see a good way forward after now two appearantly failed attempts. > > Btw, I'm fine reverting the patch but then what's the solution here? I think we should (longer term) get rid of the overloaded meanings and uses of subregs. One fairly simple thing is to make a new rtx code "bit_cast" (or is there a nice short more traditional name for it?) But that is not the core problem we had here. The behaviour of the generic parts of the compiler was changed, without testing if that works on other targets but x86. That is an understandable mistake, it takes some experience to know where the morasses are. But this change should have been accompanied by testcases exercising the changed code. We would have clearly seen there are issues then, simply by watching gcc-testresults@ (and/or maintainers are on top of the test results anyway). Also, if there were testcases for this, we could have some confidence that a change in this area is robust. Segher p.s. Very unrelated... Should we have __builtin_bit_cast for C as well? Is there any reason this could not work?