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 147D23858D1E for ; Tue, 2 May 2023 21:57:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 147D23858D1E 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 342LtEUb030395; Tue, 2 May 2023 16:55:15 -0500 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 342LtB9r030393; Tue, 2 May 2023 16:55:11 -0500 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Tue, 2 May 2023 16:55:10 -0500 From: Segher Boessenkool To: Roger Sayle Cc: "'Paul Koning'" , "'Jeff Law'" , "'GCC Patches'" Subject: Re: [committed] Convert xstormy16 to LRA Message-ID: <20230502215509.GC19790@gate.crashing.org> References: <009601d97c85$de708170$9b518450$@nextmovesoftware.com> <0FD01909-1F42-4E50-8D0D-2EDB80C34C0A@comcast.net> <021801d97cf8$9fc91770$df5b4650$@nextmovesoftware.com> <20230502134928.GA19790@gate.crashing.org> <008101d97d12$12068200$36138600$@nextmovesoftware.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <008101d97d12$12068200$36138600$@nextmovesoftware.com> 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_SHORT,SPF_HELO_PASS,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE 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 Tue, May 02, 2023 at 05:20:49PM +0100, Roger Sayle wrote: > On 02 May 2023 14:49, Segher Boessenkool wrote: > Then combine inserts an additional copy: Combine makes sure a pseudo-to-pseudo move remains. Without that, combine will seize part of RA's job, and butcher it. It has always done that, but the 2-2 combine patches made it clearer than before. > (insn 17 16 18 2 (clobber (reg/v:SI 26 [ x ])) "../../shiftsi.c":4:41 -1 > (nil)) > (insn 18 17 19 2 (set (subreg:HI (reg/v:SI 26 [ x ]) 0) > (reg:HI 30)) "../../shiftsi.c":4:41 6 {movhi_internal} > (nil)) > (insn 19 18 3 2 (set (subreg:HI (reg/v:SI 26 [ x ]) 2) > (reg:HI 31 [+2 ])) "../../shiftsi.c":4:41 6 {movhi_internal} > (nil)) > I don't think it's a problem/fault with the machine description, but > how the clobber above is being interpreted by the different register > allocators. Insn 17 is trivially dead code and should have been removed. It arguably should not have been created in the first place. Why do we do that, what is the purpose? > Back in August 2022, I submitted a x86_64 backend patch that used a > peephole2 to eliminate this type of (subreg-lower) generated clobber: > https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599419.html > but Uros was hesitant to accept this change, without an RTL expert > explaining why the clobbers were being generated in the first place. It makes sure any previous value in it is regarded as dead, but since insns 18 and 19 clearly write to the whole register anyway, this is just extra work to later undo. Or not undo, which is the problem here :-/ > Like my x86_64 patch above, this issue could also probably be cleaned > up by a peephole2 for xstormy16 that straddled/removed the clobber. Or simply not create such useless clobbers in the first place? > My simplistic summary is that when a backend lowers a multi-word > instruction with a splitter, it (typically) does so without introducing > a clobber. If the clobbers generated by the middle-end's automatic > lowering confuse the register allocator, then this is more efficient. > Ideally, such clobbers should be avoided (if possible) and/or LRA > improved to understand that they don't actually cause an allocno > conflict. [but I'm in no way a register allocation expert]. Yup, I agree with all that. We should not create dead code, and not be confused by dead code either :-) Segher