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 7A3F63858C52 for ; Tue, 10 Jan 2023 10:48:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7A3F63858C52 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 28E314B3; Tue, 10 Jan 2023 02:48:50 -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 8DDBA3F587; Tue, 10 Jan 2023 02:48:07 -0800 (PST) From: Richard Sandiford To: Uros Bizjak Mail-Followup-To: Uros Bizjak ,GCC Patches , Roger Sayle , richard.sandiford@arm.com Cc: GCC Patches , Roger Sayle Subject: Re: [x86 PATCH] PR rtl-optimization/107991: peephole2 to tweak register allocation. References: <011401d9243b$3782ce10$a6886a30$@nextmovesoftware.com> Date: Tue, 10 Jan 2023 10:48:06 +0000 In-Reply-To: (Uros Bizjak's message of "Mon, 9 Jan 2023 17:02:58 +0100") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-32.0 required=5.0 tests=BAYES_00,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,KAM_SHORT,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: Uros Bizjak writes: > On Mon, Jan 9, 2023 at 4:01 PM Roger Sayle wrote: >> >> >> This patch addresses PR rtl-optimization/107991, which is a P2 regression >> where GCC currently requires more "mov" instructions than GCC 7. >> >> The x86's two address ISA creates some interesting challenges for reload. >> For example, the tricky "x = y - x" usually needs to be implemented on x86 >> as >> >> tmp = x >> x = y >> x -= tmp >> >> where a scratch register and two mov's are required to work around >> the lack of a subf (subtract from) or rsub (reverse subtract) insn. >> >> Not uncommonly, if y is dead after this subtraction, register allocation >> can be improved by clobbering y. >> >> y -= x >> x = y >> >> For the testcase in PR 107991, things are slightly more complicated, >> where y is not itself dead, but is assigned from (i.e. equivalent to) >> a value that is dead. Hence we have something like: >> >> y = z >> x = y - x >> >> so, GCC's reload currently generates the expected shuffle (as y is live): >> >> y = z >> tmp = x >> x = y >> x -= tmp >> >> but we can use a peephole2 that understands that y and z are equivalent, >> and that z is dead, to produce the shorter sequence: >> >> y = z >> z -= x >> x = z >> >> In practice, for the new testcase from PR 107991, which before produced: >> >> foo: movl %edx, %ecx >> movl %esi, %edx >> movl %esi, %eax >> subl %ecx, %edx >> testb %dil, %dil >> cmovne %edx, %eax >> ret >> >> with this patch/peephole2 we now produce the much improved: >> >> foo: movl %esi, %eax >> subl %edx, %esi >> testb %dil, %dil >> cmovne %esi, %eax >> ret >> >> >> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap >> and make -k check, both with and without --target_board=unix{-m32}, >> with no new failures. Ok for mainline? > > Looking at the PR, it looks to me that Richard S (CC'd) wants to solve > this issue in the register allocator. This would be preferred > (compared to a very specialized peephole2), since peephole2 pass comes > very late in the game, so one freed register does not contribute to > lower the register pressure at all. Yeah, I think there are three issues that would all be good to fix: (1) the fwprop regression (2) the unhelpful pre-RA move (3) the RA handling of what it sees now I had a look last year to see where (1) was coming from, and it turned out to be bad bookkeeping during a recursive walk. I've got a couple of competing ideas for how to fix it, but I've not had time to work on it since then, sorry. > Peephole2 should be used to clean after reload only in rare cases when > target ISA prevents generic solution. From your description, a generic > solution would benefit all targets with destructive subtraction (or > perhaps also for other noncommutative operations). Yeah, agree that peepholes aren't the best fit here. The problem could occur with instructions that are too far apart to be peepholed. Thanks, Richard > So, please coordinate with Richard S regarding this issue. > > Thanks, > Uros. > >> >> >> 2023-01-09 Roger Sayle >> >> gcc/ChangeLog >> PR rtl-optimization/107991 >> * config/i386/i386.md (peephole2): New peephole2 to avoid register >> shuffling before a subtraction, after a register-to-register move. >> >> gcc/testsuite/ChangeLog >> PR rtl-optimization/107991 >> * gcc.target/i386/pr107991.c: New test case. >> >> >> Thanks in advance, >> Roger >> -- >>