| Message ID | 003f01dc810f$f7d869c0$e7893d40$@nextmovesoftware.com |
|---|---|
| State | New |
| Headers |
Return-Path: <gcc-patches-bounces~patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id 91D974BA2E1F for <patchwork@sourceware.org>; Fri, 9 Jan 2026 02:31:35 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 91D974BA2E1F Authentication-Results: sourceware.org; dkim=fail reason="signature verification failed" (2048-bit key, unprotected) header.d=nextmovesoftware.com header.i=@nextmovesoftware.com header.a=rsa-sha256 header.s=default header.b=dDx6eNEV X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from server.nextmovesoftware.com (server.nextmovesoftware.com [69.48.154.134]) by sourceware.org (Postfix) with ESMTPS id 4FA414BA2E06 for <gcc-patches@gcc.gnu.org>; Fri, 9 Jan 2026 02:30:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4FA414BA2E06 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=nextmovesoftware.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=nextmovesoftware.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 4FA414BA2E06 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=69.48.154.134 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1767925851; cv=none; b=BWh4CMKi/xVl94fnYM/JMCrRQlMvANmpsW7/VRWnA+4IcFO7PdKgUbXYaw18+EU5Qw1eqisO82fffeV90PkTgwFvZ5HAaauRn9KZT4pbiV8VLyAwdRFZ9O18mWMs0CH3vOq8nUCziFEy04kHOs/CQEYvSEqoD+zqyhnJmC3eTgY= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1767925851; c=relaxed/simple; bh=7DISW7F9pkKm/XZlKNpWyrVArxdh91fzfKNhViBbA8c=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=LhveIxu+BNjqjf2ziJN65IysfvJksSR4hxDgonFERPVgHTSO0R0Num8EY5JUzNS3ebzTcOLoZhwJFBwWBC684Swy1oBrK2xcBgz5hUpcsHdqAC8SgTDMv0fkvjfkqv4ivtIym92oQMwGBQrX3lWgNlSQv05Jc7pnPwgS0/Y3eoA= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4FA414BA2E06 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=nextmovesoftware.com; s=default; h=Content-Type:MIME-Version:Message-ID: Date:Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:In-Reply-To:References:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=Nzc/9R44tpcijesAl/TjzvqcwmCiQHth+GP4RaY/bho=; b=dDx6eNEVaD/J2Jb9bSgZKr76h5 rcacTXqzFnxAmni/VqoCdSkjtKIVqfph05s+O8lrlJ2oSxVgQnaPljytUEjUWREqn6k8vYTQcnADC Pd13aTp1TV5O2xeGMadoQX4zFB6oP1M7jkhlLOIZ9ACwi327PMwj4D7eO0ru1H5+xSMGKue6dVK/U C1StlI/0nNqXL/AYzHmnVa6X0RBx7r+RLzvXOlRb9x984rZu3c25T04lepXyPKR0l/PdIhlhcTjPs HZz0C4Nb+ifleNmd26LRbHGXDJ2vS0UFq6ZqHaE8S85lyKC8GQMo6cBkD1lMTVQqHkysQrZextr5A uv6njKLg==; Received: from [168.86.198.35] (port=64427 helo=Dell) by server.nextmovesoftware.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.99) (envelope-from <roger@nextmovesoftware.com>) id 1ve2H3-00000004DUh-023s; Thu, 08 Jan 2026 21:30:49 -0500 From: "Roger Sayle" <roger@nextmovesoftware.com> To: "'GCC Patches'" <gcc-patches@gcc.gnu.org> Cc: "'Segher Boessenkool'" <segher@kernel.crashing.org> Subject: [combine] PR middle-end/123236: Simplify (int)((long long)x >> 4) Date: Fri, 9 Jan 2026 02:30:47 -0000 Message-ID: <003f01dc810f$f7d869c0$e7893d40$@nextmovesoftware.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_NextPart_000_0040_01DC810F.F7D869C0" X-Mailer: Microsoft Outlook 16.0 Thread-Index: AdyBDo+zc/gaLTWKQ16BtmPB9fwv0Q== Content-Language: en-gb X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - server.nextmovesoftware.com X-AntiAbuse: Original Domain - gcc.gnu.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - nextmovesoftware.com X-Get-Message-Sender-Via: server.nextmovesoftware.com: authenticated_id: roger@nextmovesoftware.com X-Authenticated-Sender: server.nextmovesoftware.com: roger@nextmovesoftware.com X-Source: X-Source-Args: X-Source-Dir: X-Spam-Status: No, score=-12.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED, RCVD_IN_VALIDITY_SAFE_BLOCKED, SPF_HELO_NONE, SPF_PASS, TXREP, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> Errors-To: gcc-patches-bounces~patchwork=sourceware.org@gcc.gnu.org |
| Series |
[combine] PR middle-end/123236: Simplify (int)((long long)x >> 4)
|
|
Commit Message
Roger Sayle
Jan. 9, 2026, 2:30 a.m. UTC
This patch addresses a code quality regression on x86_64 related to
PR 123236. That original PR (and the related PR 101266) concern tree
level optimizations, where this problem should also be fixed, but it
also reveals a regression in the RTL optimizers.
A motivating test case (on x86_64) is:
unsigned int foo(unsigned int a) {
unsigned long long t = a;
return t >> 4;
}
which with -O2 currently generates (2 64-bit shifts):
foo: movq %rdi, %rax
salq $32, %rax
shrq $36, %rax
ret
with this patch, we now generate (1 32-bit shift):
foo: movl %edi, %eax
shrl $4, %eax
ret
which matches what GCC generated prior to GCC 11.
Likewise, for the signed (SIGN_EXTRACT/SIGN_EXTEND) case:
int bar(int a) {
long long t = a;
return t >> 4;
}
Before:
bar: movslq %edi, %rax
sarq $4, %rax
ret
After:
bar: movl %edi, %eax
sarl $4, %eax
ret
The underlying cause of the RTL-level regression is that some
RTL expressions that were previously expressed as {ZERO,SIGN}_EXTEND
are now sometimes represented as the equivalent {ZERO,SIGN}_EXTRACT,
and that not all simplifications of TRUNCATE({ZERO,SIGN}_EXTEND) are
implemented for TRUNCATE({ZERO,SIGN}_EXTRACT).
Thanks to Segher, simplify_rtx does handle some truncations of extracts,
see https://gcc.gnu.org/pipermail/gcc-patches/2016-November/463629.html
but unfortunately this code (and subsequent tweaks) doesn't quite match
the cases that we care about.
Previously:
Trying 6, 7 -> 8:
6: r103:DI=sign_extend(r105:SI)
REG_DEAD r105:SI
7: {r104:DI=r103:DI>>0x4;clobber flags:CC;}
REG_DEAD r103:DI
REG_UNUSED flags:CC
8: r102:SI=r104:DI#0
REG_DEAD r104:DI
Failed to match this instruction:
(set (subreg:DI (reg:SI 102 [ _4 ]) 0)
(zero_extend:DI (subreg:SI (sign_extract:DI (reg:SI 105 [ aD.2962 ])
(const_int 28 [0x1c])
(const_int 4 [0x4])) 0)))
With the tweaks below, this becomes:
Trying 6, 7 -> 8:
6: r103:DI=sign_extend(r105:SI)
REG_DEAD r105:SI
7: {r104:DI=r103:DI>>0x4;clobber flags:CC;}
REG_DEAD r103:DI
REG_UNUSED flags:CC
8: r102:SI=r104:DI#0
REG_DEAD r104:DI
Successfully matched this instruction:
(set (reg:SI 102 [ _4 ])
(ashiftrt:SI (reg:SI 105 [ aD.2962 ])
(const_int 4 [0x4])))
allowing combination of insns 6, 7 and 8
original costs 4 + 4 + 4 = 12
replacement cost 4
deferring deletion of insn with uid = 7.
deferring deletion of insn with uid = 6.
modifying insn i3 8: {r102:SI=r105:SI>>0x4;clobber flags:CC;}
REG_UNUSED flags:CC
REG_DEAD r105:SI
deferring rescan insn with uid = 8.
This requires two minor tweaks. The first is that the paradoxical
subreg assignment (set (subreg:DI (reg:SI 102) 0) (expr:DI)) can
(sometimes) be simplified to (set (reg:SI 102) (truncate:SI (expr:DI))
especially if the (truncate:SI (expr:DI)) can itself be simplified.
The second is that (truncate:SI (sign_extract:DI (reg:SI 102) x y))
slips through the existing simplify_truncation transformations as the
the {zero,sign}_extract has a different mode to its first operand.
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?
2026-01-09 Roger Sayle <roger@nextmovesoftware.com>
gcc/ChangeLog
PR rtl-optimization/123236
* combine.cc (simplify_set): Attempt to simplify SETs where the
destination is a low-part paradoxical subreg between scalar integer
modes.
* simplify-rtx.cc (simplify_context::simplify_truncation): Handle
cases where a ZERO_EXTRACT or SIGN_EXTRACT has a different mode
to (at least as wide as) its first operand.
gcc/testsuite/ChangeLog
PR rtl-optimization/123236
* gcc.target/i386/pr123236.c: New test case.
Thanks in advance,
Roger
--
Comments
On 1/8/2026 7:30 PM, Roger Sayle wrote: > This patch addresses a code quality regression on x86_64 related to > PR 123236. That original PR (and the related PR 101266) concern tree > level optimizations, where this problem should also be fixed, but it > also reveals a regression in the RTL optimizers. > > A motivating test case (on x86_64) is: > > unsigned int foo(unsigned int a) { > unsigned long long t = a; > return t >> 4; > } > > which with -O2 currently generates (2 64-bit shifts): > > foo: movq %rdi, %rax > salq $32, %rax > shrq $36, %rax > ret > > with this patch, we now generate (1 32-bit shift): > > foo: movl %edi, %eax > shrl $4, %eax > ret > > which matches what GCC generated prior to GCC 11. > Likewise, for the signed (SIGN_EXTRACT/SIGN_EXTEND) case: > > int bar(int a) { > long long t = a; > return t >> 4; > } > > Before: > bar: movslq %edi, %rax > sarq $4, %rax > ret > > After: > bar: movl %edi, %eax > sarl $4, %eax > ret > > The underlying cause of the RTL-level regression is that some > RTL expressions that were previously expressed as {ZERO,SIGN}_EXTEND > are now sometimes represented as the equivalent {ZERO,SIGN}_EXTRACT, > and that not all simplifications of TRUNCATE({ZERO,SIGN}_EXTEND) are > implemented for TRUNCATE({ZERO,SIGN}_EXTRACT). > > Thanks to Segher, simplify_rtx does handle some truncations of extracts, > see https://gcc.gnu.org/pipermail/gcc-patches/2016-November/463629.html > but unfortunately this code (and subsequent tweaks) doesn't quite match > the cases that we care about. > > Previously: > Trying 6, 7 -> 8: > 6: r103:DI=sign_extend(r105:SI) > REG_DEAD r105:SI > 7: {r104:DI=r103:DI>>0x4;clobber flags:CC;} > REG_DEAD r103:DI > REG_UNUSED flags:CC > 8: r102:SI=r104:DI#0 > REG_DEAD r104:DI > Failed to match this instruction: > (set (subreg:DI (reg:SI 102 [ _4 ]) 0) > (zero_extend:DI (subreg:SI (sign_extract:DI (reg:SI 105 [ aD.2962 ]) > (const_int 28 [0x1c]) > (const_int 4 [0x4])) 0))) > > With the tweaks below, this becomes: > Trying 6, 7 -> 8: > 6: r103:DI=sign_extend(r105:SI) > REG_DEAD r105:SI > 7: {r104:DI=r103:DI>>0x4;clobber flags:CC;} > REG_DEAD r103:DI > REG_UNUSED flags:CC > 8: r102:SI=r104:DI#0 > REG_DEAD r104:DI > Successfully matched this instruction: > (set (reg:SI 102 [ _4 ]) > (ashiftrt:SI (reg:SI 105 [ aD.2962 ]) > (const_int 4 [0x4]))) > allowing combination of insns 6, 7 and 8 > original costs 4 + 4 + 4 = 12 > replacement cost 4 > deferring deletion of insn with uid = 7. > deferring deletion of insn with uid = 6. > modifying insn i3 8: {r102:SI=r105:SI>>0x4;clobber flags:CC;} > REG_UNUSED flags:CC > REG_DEAD r105:SI > deferring rescan insn with uid = 8. > > This requires two minor tweaks. The first is that the paradoxical > subreg assignment (set (subreg:DI (reg:SI 102) 0) (expr:DI)) can > (sometimes) be simplified to (set (reg:SI 102) (truncate:SI (expr:DI)) > especially if the (truncate:SI (expr:DI)) can itself be simplified. > The second is that (truncate:SI (sign_extract:DI (reg:SI 102) x y)) > slips through the existing simplify_truncation transformations as the > the {zero,sign}_extract has a different mode to its first operand. > > 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? > > 2026-01-09 Roger Sayle <roger@nextmovesoftware.com> > > gcc/ChangeLog > PR rtl-optimization/123236 > * combine.cc (simplify_set): Attempt to simplify SETs where the > destination is a low-part paradoxical subreg between scalar integer > modes. > * simplify-rtx.cc (simplify_context::simplify_truncation): Handle > cases where a ZERO_EXTRACT or SIGN_EXTRACT has a different mode > to (at least as wide as) its first operand. > > gcc/testsuite/ChangeLog > PR rtl-optimization/123236 > * gcc.target/i386/pr123236.c: New test case. So in simplify_truncation you dropped the REG_P (XEXP (op, 0)) check. Was that intentional? I guess since you don't ultimately change the mode of the truncated object that's probably OK. So as long as it was intentional, I won't object to it. So if the removal was intentional, then OK as-is. If it was a goof, then OK with the check restored. jeff
Jeffrey Law <jefflaw@qti.qualcomm.com> writes: > On 1/8/2026 7:30 PM, Roger Sayle wrote: >> This patch addresses a code quality regression on x86_64 related to >> PR 123236. That original PR (and the related PR 101266) concern tree >> level optimizations, where this problem should also be fixed, but it >> also reveals a regression in the RTL optimizers. >> >> A motivating test case (on x86_64) is: >> >> unsigned int foo(unsigned int a) { >> unsigned long long t = a; >> return t >> 4; >> } >> >> which with -O2 currently generates (2 64-bit shifts): >> >> foo: movq %rdi, %rax >> salq $32, %rax >> shrq $36, %rax >> ret >> >> with this patch, we now generate (1 32-bit shift): >> >> foo: movl %edi, %eax >> shrl $4, %eax >> ret >> >> which matches what GCC generated prior to GCC 11. >> Likewise, for the signed (SIGN_EXTRACT/SIGN_EXTEND) case: >> >> int bar(int a) { >> long long t = a; >> return t >> 4; >> } >> >> Before: >> bar: movslq %edi, %rax >> sarq $4, %rax >> ret >> >> After: >> bar: movl %edi, %eax >> sarl $4, %eax >> ret >> >> The underlying cause of the RTL-level regression is that some >> RTL expressions that were previously expressed as {ZERO,SIGN}_EXTEND >> are now sometimes represented as the equivalent {ZERO,SIGN}_EXTRACT, >> and that not all simplifications of TRUNCATE({ZERO,SIGN}_EXTEND) are >> implemented for TRUNCATE({ZERO,SIGN}_EXTRACT). >> >> Thanks to Segher, simplify_rtx does handle some truncations of extracts, >> see https://gcc.gnu.org/pipermail/gcc-patches/2016-November/463629.html >> but unfortunately this code (and subsequent tweaks) doesn't quite match >> the cases that we care about. >> >> Previously: >> Trying 6, 7 -> 8: >> 6: r103:DI=sign_extend(r105:SI) >> REG_DEAD r105:SI >> 7: {r104:DI=r103:DI>>0x4;clobber flags:CC;} >> REG_DEAD r103:DI >> REG_UNUSED flags:CC >> 8: r102:SI=r104:DI#0 >> REG_DEAD r104:DI >> Failed to match this instruction: >> (set (subreg:DI (reg:SI 102 [ _4 ]) 0) >> (zero_extend:DI (subreg:SI (sign_extract:DI (reg:SI 105 [ aD.2962 ]) >> (const_int 28 [0x1c]) >> (const_int 4 [0x4])) 0))) >> >> With the tweaks below, this becomes: >> Trying 6, 7 -> 8: >> 6: r103:DI=sign_extend(r105:SI) >> REG_DEAD r105:SI >> 7: {r104:DI=r103:DI>>0x4;clobber flags:CC;} >> REG_DEAD r103:DI >> REG_UNUSED flags:CC >> 8: r102:SI=r104:DI#0 >> REG_DEAD r104:DI >> Successfully matched this instruction: >> (set (reg:SI 102 [ _4 ]) >> (ashiftrt:SI (reg:SI 105 [ aD.2962 ]) >> (const_int 4 [0x4]))) >> allowing combination of insns 6, 7 and 8 >> original costs 4 + 4 + 4 = 12 >> replacement cost 4 >> deferring deletion of insn with uid = 7. >> deferring deletion of insn with uid = 6. >> modifying insn i3 8: {r102:SI=r105:SI>>0x4;clobber flags:CC;} >> REG_UNUSED flags:CC >> REG_DEAD r105:SI >> deferring rescan insn with uid = 8. >> >> This requires two minor tweaks. The first is that the paradoxical >> subreg assignment (set (subreg:DI (reg:SI 102) 0) (expr:DI)) can >> (sometimes) be simplified to (set (reg:SI 102) (truncate:SI (expr:DI)) >> especially if the (truncate:SI (expr:DI)) can itself be simplified. >> The second is that (truncate:SI (sign_extract:DI (reg:SI 102) x y)) >> slips through the existing simplify_truncation transformations as the >> the {zero,sign}_extract has a different mode to its first operand. >> >> 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? >> >> 2026-01-09 Roger Sayle <roger@nextmovesoftware.com> >> >> gcc/ChangeLog >> PR rtl-optimization/123236 >> * combine.cc (simplify_set): Attempt to simplify SETs where the >> destination is a low-part paradoxical subreg between scalar integer >> modes. >> * simplify-rtx.cc (simplify_context::simplify_truncation): Handle >> cases where a ZERO_EXTRACT or SIGN_EXTRACT has a different mode >> to (at least as wide as) its first operand. >> >> gcc/testsuite/ChangeLog >> PR rtl-optimization/123236 >> * gcc.target/i386/pr123236.c: New test case. > So in simplify_truncation you dropped the REG_P (XEXP (op, 0)) check. > Was that intentional? I guess since you don't ultimately change the > mode of the truncated object that's probably OK. So as long as it was > intentional, I won't object to it. Also, I wonder how friendly this would be on RISC targets. The use of simplify_gen_unary... > + /* If we have (set (subreg:M (reg:N DEST) SRC) with M wider than N, > + we have a paradoxical subreg destination, such as created by the > + clause above, check if we can simplify (subreg:N SRC) to eliminate > + the (paradoxical) subreg. */ > + else if (GET_CODE (dest) == SUBREG > + && subreg_lowpart_p (dest) > + && paradoxical_subreg_p (dest) > + && SCALAR_INT_MODE_P (mode) > + && SCALAR_INT_MODE_P (GET_MODE (SUBREG_REG (dest)))) > + { > + rtx tmp = simplify_gen_unary (TRUNCATE, GET_MODE (SUBREG_REG (dest)), > + src, mode); ...here means that we'd narrow the operation even if no simplification of the rhs is possible. So, for example, I can imagine this trying to create a QImode operation on targets that only provide SImode or DImode operations. This seems like something that would be worth trying if a recog of the original pattern fails, but maybe isn't something that we should commit to up-front, before any recog has been attempted. (BTW, the simplify_lowpart_p check is redundant with the paradoxical_subreg_p check.) Thanks, Richard > + if (tmp) > + { > + SUBST (SET_DEST (x), SUBREG_REG (dest)); > + SUBST (SET_SRC (x), tmp); > + dest = SET_DEST (x); > + src = SET_SRC (x); > + } > + } > +
On 5/4/2026 10:46 AM, Richard Sandiford wrote: > WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros. > > Jeffrey Law <jefflaw@qti.qualcomm.com> writes: >> On 1/8/2026 7:30 PM, Roger Sayle wrote: >>> This patch addresses a code quality regression on x86_64 related to >>> PR 123236. That original PR (and the related PR 101266) concern tree >>> level optimizations, where this problem should also be fixed, but it >>> also reveals a regression in the RTL optimizers. >>> >>> A motivating test case (on x86_64) is: >>> >>> unsigned int foo(unsigned int a) { >>> unsigned long long t = a; >>> return t >> 4; >>> } >>> >>> which with -O2 currently generates (2 64-bit shifts): >>> >>> foo: movq %rdi, %rax >>> salq $32, %rax >>> shrq $36, %rax >>> ret >>> >>> with this patch, we now generate (1 32-bit shift): >>> >>> foo: movl %edi, %eax >>> shrl $4, %eax >>> ret >>> >>> which matches what GCC generated prior to GCC 11. >>> Likewise, for the signed (SIGN_EXTRACT/SIGN_EXTEND) case: >>> >>> int bar(int a) { >>> long long t = a; >>> return t >> 4; >>> } >>> >>> Before: >>> bar: movslq %edi, %rax >>> sarq $4, %rax >>> ret >>> >>> After: >>> bar: movl %edi, %eax >>> sarl $4, %eax >>> ret >>> >>> The underlying cause of the RTL-level regression is that some >>> RTL expressions that were previously expressed as {ZERO,SIGN}_EXTEND >>> are now sometimes represented as the equivalent {ZERO,SIGN}_EXTRACT, >>> and that not all simplifications of TRUNCATE({ZERO,SIGN}_EXTEND) are >>> implemented for TRUNCATE({ZERO,SIGN}_EXTRACT). >>> >>> Thanks to Segher, simplify_rtx does handle some truncations of extracts, >>> see https://gcc.gnu.org/pipermail/gcc-patches/2016-November/463629.html >>> but unfortunately this code (and subsequent tweaks) doesn't quite match >>> the cases that we care about. >>> >>> Previously: >>> Trying 6, 7 -> 8: >>> 6: r103:DI=sign_extend(r105:SI) >>> REG_DEAD r105:SI >>> 7: {r104:DI=r103:DI>>0x4;clobber flags:CC;} >>> REG_DEAD r103:DI >>> REG_UNUSED flags:CC >>> 8: r102:SI=r104:DI#0 >>> REG_DEAD r104:DI >>> Failed to match this instruction: >>> (set (subreg:DI (reg:SI 102 [ _4 ]) 0) >>> (zero_extend:DI (subreg:SI (sign_extract:DI (reg:SI 105 [ aD.2962 ]) >>> (const_int 28 [0x1c]) >>> (const_int 4 [0x4])) 0))) >>> >>> With the tweaks below, this becomes: >>> Trying 6, 7 -> 8: >>> 6: r103:DI=sign_extend(r105:SI) >>> REG_DEAD r105:SI >>> 7: {r104:DI=r103:DI>>0x4;clobber flags:CC;} >>> REG_DEAD r103:DI >>> REG_UNUSED flags:CC >>> 8: r102:SI=r104:DI#0 >>> REG_DEAD r104:DI >>> Successfully matched this instruction: >>> (set (reg:SI 102 [ _4 ]) >>> (ashiftrt:SI (reg:SI 105 [ aD.2962 ]) >>> (const_int 4 [0x4]))) >>> allowing combination of insns 6, 7 and 8 >>> original costs 4 + 4 + 4 = 12 >>> replacement cost 4 >>> deferring deletion of insn with uid = 7. >>> deferring deletion of insn with uid = 6. >>> modifying insn i3 8: {r102:SI=r105:SI>>0x4;clobber flags:CC;} >>> REG_UNUSED flags:CC >>> REG_DEAD r105:SI >>> deferring rescan insn with uid = 8. >>> >>> This requires two minor tweaks. The first is that the paradoxical >>> subreg assignment (set (subreg:DI (reg:SI 102) 0) (expr:DI)) can >>> (sometimes) be simplified to (set (reg:SI 102) (truncate:SI (expr:DI)) >>> especially if the (truncate:SI (expr:DI)) can itself be simplified. >>> The second is that (truncate:SI (sign_extract:DI (reg:SI 102) x y)) >>> slips through the existing simplify_truncation transformations as the >>> the {zero,sign}_extract has a different mode to its first operand. >>> >>> 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? >>> >>> 2026-01-09 Roger Sayle <roger@nextmovesoftware.com> >>> >>> gcc/ChangeLog >>> PR rtl-optimization/123236 >>> * combine.cc (simplify_set): Attempt to simplify SETs where the >>> destination is a low-part paradoxical subreg between scalar integer >>> modes. >>> * simplify-rtx.cc (simplify_context::simplify_truncation): Handle >>> cases where a ZERO_EXTRACT or SIGN_EXTRACT has a different mode >>> to (at least as wide as) its first operand. >>> >>> gcc/testsuite/ChangeLog >>> PR rtl-optimization/123236 >>> * gcc.target/i386/pr123236.c: New test case. >> So in simplify_truncation you dropped the REG_P (XEXP (op, 0)) check. >> Was that intentional? I guess since you don't ultimately change the >> mode of the truncated object that's probably OK. So as long as it was >> intentional, I won't object to it. > Also, I wonder how friendly this would be on RISC targets. The use > of simplify_gen_unary... Are you referring to simplify_gen_unary failing and returning NULL or is the resultant code we generate RISC friendly? For the latter, we just potentially miss an optimization, the former could be an ICE/segfault scenario. Jeff
Jeffrey Law <jefflaw@qti.qualcomm.com> writes: > On 5/4/2026 10:46 AM, Richard Sandiford wrote: >> Jeffrey Law <jefflaw@qti.qualcomm.com> writes: >>> On 1/8/2026 7:30 PM, Roger Sayle wrote: >>>> This patch addresses a code quality regression on x86_64 related to >>>> PR 123236. That original PR (and the related PR 101266) concern tree >>>> level optimizations, where this problem should also be fixed, but it >>>> also reveals a regression in the RTL optimizers. >>>> >>>> A motivating test case (on x86_64) is: >>>> >>>> unsigned int foo(unsigned int a) { >>>> unsigned long long t = a; >>>> return t >> 4; >>>> } >>>> >>>> which with -O2 currently generates (2 64-bit shifts): >>>> >>>> foo: movq %rdi, %rax >>>> salq $32, %rax >>>> shrq $36, %rax >>>> ret >>>> >>>> with this patch, we now generate (1 32-bit shift): >>>> >>>> foo: movl %edi, %eax >>>> shrl $4, %eax >>>> ret >>>> >>>> which matches what GCC generated prior to GCC 11. >>>> Likewise, for the signed (SIGN_EXTRACT/SIGN_EXTEND) case: >>>> >>>> int bar(int a) { >>>> long long t = a; >>>> return t >> 4; >>>> } >>>> >>>> Before: >>>> bar: movslq %edi, %rax >>>> sarq $4, %rax >>>> ret >>>> >>>> After: >>>> bar: movl %edi, %eax >>>> sarl $4, %eax >>>> ret >>>> >>>> The underlying cause of the RTL-level regression is that some >>>> RTL expressions that were previously expressed as {ZERO,SIGN}_EXTEND >>>> are now sometimes represented as the equivalent {ZERO,SIGN}_EXTRACT, >>>> and that not all simplifications of TRUNCATE({ZERO,SIGN}_EXTEND) are >>>> implemented for TRUNCATE({ZERO,SIGN}_EXTRACT). >>>> >>>> Thanks to Segher, simplify_rtx does handle some truncations of extracts, >>>> see https://gcc.gnu.org/pipermail/gcc-patches/2016-November/463629.html >>>> but unfortunately this code (and subsequent tweaks) doesn't quite match >>>> the cases that we care about. >>>> >>>> Previously: >>>> Trying 6, 7 -> 8: >>>> 6: r103:DI=sign_extend(r105:SI) >>>> REG_DEAD r105:SI >>>> 7: {r104:DI=r103:DI>>0x4;clobber flags:CC;} >>>> REG_DEAD r103:DI >>>> REG_UNUSED flags:CC >>>> 8: r102:SI=r104:DI#0 >>>> REG_DEAD r104:DI >>>> Failed to match this instruction: >>>> (set (subreg:DI (reg:SI 102 [ _4 ]) 0) >>>> (zero_extend:DI (subreg:SI (sign_extract:DI (reg:SI 105 [ aD.2962 ]) >>>> (const_int 28 [0x1c]) >>>> (const_int 4 [0x4])) 0))) >>>> >>>> With the tweaks below, this becomes: >>>> Trying 6, 7 -> 8: >>>> 6: r103:DI=sign_extend(r105:SI) >>>> REG_DEAD r105:SI >>>> 7: {r104:DI=r103:DI>>0x4;clobber flags:CC;} >>>> REG_DEAD r103:DI >>>> REG_UNUSED flags:CC >>>> 8: r102:SI=r104:DI#0 >>>> REG_DEAD r104:DI >>>> Successfully matched this instruction: >>>> (set (reg:SI 102 [ _4 ]) >>>> (ashiftrt:SI (reg:SI 105 [ aD.2962 ]) >>>> (const_int 4 [0x4]))) >>>> allowing combination of insns 6, 7 and 8 >>>> original costs 4 + 4 + 4 = 12 >>>> replacement cost 4 >>>> deferring deletion of insn with uid = 7. >>>> deferring deletion of insn with uid = 6. >>>> modifying insn i3 8: {r102:SI=r105:SI>>0x4;clobber flags:CC;} >>>> REG_UNUSED flags:CC >>>> REG_DEAD r105:SI >>>> deferring rescan insn with uid = 8. >>>> >>>> This requires two minor tweaks. The first is that the paradoxical >>>> subreg assignment (set (subreg:DI (reg:SI 102) 0) (expr:DI)) can >>>> (sometimes) be simplified to (set (reg:SI 102) (truncate:SI (expr:DI)) >>>> especially if the (truncate:SI (expr:DI)) can itself be simplified. >>>> The second is that (truncate:SI (sign_extract:DI (reg:SI 102) x y)) >>>> slips through the existing simplify_truncation transformations as the >>>> the {zero,sign}_extract has a different mode to its first operand. >>>> >>>> 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? >>>> >>>> 2026-01-09 Roger Sayle <roger@nextmovesoftware.com> >>>> >>>> gcc/ChangeLog >>>> PR rtl-optimization/123236 >>>> * combine.cc (simplify_set): Attempt to simplify SETs where the >>>> destination is a low-part paradoxical subreg between scalar integer >>>> modes. >>>> * simplify-rtx.cc (simplify_context::simplify_truncation): Handle >>>> cases where a ZERO_EXTRACT or SIGN_EXTRACT has a different mode >>>> to (at least as wide as) its first operand. >>>> >>>> gcc/testsuite/ChangeLog >>>> PR rtl-optimization/123236 >>>> * gcc.target/i386/pr123236.c: New test case. >>> So in simplify_truncation you dropped the REG_P (XEXP (op, 0)) check. >>> Was that intentional? I guess since you don't ultimately change the >>> mode of the truncated object that's probably OK. So as long as it was >>> intentional, I won't object to it. >> Also, I wonder how friendly this would be on RISC targets. The use >> of simplify_gen_unary... > Are you referring to simplify_gen_unary failing and returning NULL or is > the resultant code we generate RISC friendly? For the latter, we just > potentially miss an optimization, the former could be an ICE/segfault > scenario. Yeah, it was the latter. If the code had used simplify_unary_operation instead of simplify_gen_unary, it would at least fire only when a simplification is possible. But as written, it looks like it effectively eliminates all assignments to paradoxical subregs, provided that both modes are integral. So I agree that it's only a missed optimisation concern rather than a correctness concern. But it doesn't feel like a natural canonicalisation for RISC. Thanks, Richard
Hi Richard,
The same class of bug is also present on aarch64, as an example of a RISC
target, but there it presents differently in the RTL, due to the internal
representations making less use of SUBREG.
Consider the four functions:
int sfoo(int a) {
long long t = a;
return t >> 4;
}
unsigned int ufoo(unsigned int a) {
unsigned long long t = a;
return t >> 4;
}
long long sbar(long long x)
{
__int128 t = x;
return t >> 4;
}
unsigned long long ubar(unsigned long long x)
{
unsigned __int128 t = x;
return t >> 4;
}
With -O2, GCC on aarch64 currently generates:
sfoo:
sbfx x0, x0, 4, 28
ret
ufoo:
lsr w0, w0, 4
ret
sbar:
asr x1, x0, 63
extr x0, x1, x0, 4
ret
ubar:
lsr x0, x0, 4
ret
Whereas current clang/LLVM generates (the hoped for):
sfoo:
asr w0, w0, #4
ret
ufoo:
lsr w0, w0, #4
ret
sbar:
asr x0, x0, #4
ret
ubar:
lsr x0, x0, #4
ret
So the motivation for the patch is independent of the target,
it's just that some (RISC) backends don’t suffer SUBREG headaches
to the same extent as others (such as x86 and AVR).
Roger
--
> -----Original Message-----
> From: Richard Sandiford <rdsandiford@googlemail.com>
> Sent: 04 May 2026 17:46
> To: Jeffrey Law <jefflaw@qti.qualcomm.com>
> Cc: Roger Sayle <roger@nextmovesoftware.com>; 'GCC Patches' <gcc-
> patches@gcc.gnu.org>; 'Segher Boessenkool' <segher@kernel.crashing.org>
> Subject: Re: [combine] PR middle-end/123236: Simplify (int)((long long)x >> 4)
>
> Jeffrey Law <jefflaw@qti.qualcomm.com> writes:
> > On 1/8/2026 7:30 PM, Roger Sayle wrote:
> >> This patch addresses a code quality regression on x86_64 related to
> >> PR 123236. That original PR (and the related PR 101266) concern tree
> >> level optimizations, where this problem should also be fixed, but it
> >> also reveals a regression in the RTL optimizers.
> >>
> >> A motivating test case (on x86_64) is:
> >>
> >> unsigned int foo(unsigned int a) {
> >> unsigned long long t = a;
> >> return t >> 4;
> >> }
> >>
> >> which with -O2 currently generates (2 64-bit shifts):
> >>
> >> foo: movq %rdi, %rax
> >> salq $32, %rax
> >> shrq $36, %rax
> >> ret
> >>
> >> with this patch, we now generate (1 32-bit shift):
> >>
> >> foo: movl %edi, %eax
> >> shrl $4, %eax
> >> ret
> >>
> >> which matches what GCC generated prior to GCC 11.
> >> Likewise, for the signed (SIGN_EXTRACT/SIGN_EXTEND) case:
> >>
> >> int bar(int a) {
> >> long long t = a;
> >> return t >> 4;
> >> }
> >>
> >> Before:
> >> bar: movslq %edi, %rax
> >> sarq $4, %rax
> >> ret
> >>
> >> After:
> >> bar: movl %edi, %eax
> >> sarl $4, %eax
> >> ret
> >>
> >> The underlying cause of the RTL-level regression is that some RTL
> >> expressions that were previously expressed as {ZERO,SIGN}_EXTEND are
> >> now sometimes represented as the equivalent {ZERO,SIGN}_EXTRACT, and
> >> that not all simplifications of TRUNCATE({ZERO,SIGN}_EXTEND) are
> >> implemented for TRUNCATE({ZERO,SIGN}_EXTRACT).
> >>
> >> Thanks to Segher, simplify_rtx does handle some truncations of
> >> extracts, see
> >> https://gcc.gnu.org/pipermail/gcc-patches/2016-November/463629.html
> >> but unfortunately this code (and subsequent tweaks) doesn't quite
> >> match the cases that we care about.
> >>
> >> Previously:
> >> Trying 6, 7 -> 8:
> >> 6: r103:DI=sign_extend(r105:SI)
> >> REG_DEAD r105:SI
> >> 7: {r104:DI=r103:DI>>0x4;clobber flags:CC;}
> >> REG_DEAD r103:DI
> >> REG_UNUSED flags:CC
> >> 8: r102:SI=r104:DI#0
> >> REG_DEAD r104:DI
> >> Failed to match this instruction:
> >> (set (subreg:DI (reg:SI 102 [ _4 ]) 0)
> >> (zero_extend:DI (subreg:SI (sign_extract:DI (reg:SI 105 [ aD.2962 ])
> >> (const_int 28 [0x1c])
> >> (const_int 4 [0x4])) 0)))
> >>
> >> With the tweaks below, this becomes:
> >> Trying 6, 7 -> 8:
> >> 6: r103:DI=sign_extend(r105:SI)
> >> REG_DEAD r105:SI
> >> 7: {r104:DI=r103:DI>>0x4;clobber flags:CC;}
> >> REG_DEAD r103:DI
> >> REG_UNUSED flags:CC
> >> 8: r102:SI=r104:DI#0
> >> REG_DEAD r104:DI
> >> Successfully matched this instruction:
> >> (set (reg:SI 102 [ _4 ])
> >> (ashiftrt:SI (reg:SI 105 [ aD.2962 ])
> >> (const_int 4 [0x4])))
> >> allowing combination of insns 6, 7 and 8 original costs 4 + 4 + 4 =
> >> 12 replacement cost 4 deferring deletion of insn with uid = 7.
> >> deferring deletion of insn with uid = 6.
> >> modifying insn i3 8: {r102:SI=r105:SI>>0x4;clobber flags:CC;}
> >> REG_UNUSED flags:CC
> >> REG_DEAD r105:SI
> >> deferring rescan insn with uid = 8.
> >>
> >> This requires two minor tweaks. The first is that the paradoxical
> >> subreg assignment (set (subreg:DI (reg:SI 102) 0) (expr:DI)) can
> >> (sometimes) be simplified to (set (reg:SI 102) (truncate:SI
> >> (expr:DI)) especially if the (truncate:SI (expr:DI)) can itself be simplified.
> >> The second is that (truncate:SI (sign_extract:DI (reg:SI 102) x y))
> >> slips through the existing simplify_truncation transformations as the
> >> the {zero,sign}_extract has a different mode to its first operand.
> >>
> >> 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?
> >>
> >> 2026-01-09 Roger Sayle <roger@nextmovesoftware.com>
> >>
> >> gcc/ChangeLog
> >> PR rtl-optimization/123236
> >> * combine.cc (simplify_set): Attempt to simplify SETs where the
> >> destination is a low-part paradoxical subreg between scalar integer
> >> modes.
> >> * simplify-rtx.cc (simplify_context::simplify_truncation): Handle
> >> cases where a ZERO_EXTRACT or SIGN_EXTRACT has a different mode
> >> to (at least as wide as) its first operand.
> >>
> >> gcc/testsuite/ChangeLog
> >> PR rtl-optimization/123236
> >> * gcc.target/i386/pr123236.c: New test case.
> > So in simplify_truncation you dropped the REG_P (XEXP (op, 0)) check.
> > Was that intentional? I guess since you don't ultimately change the
> > mode of the truncated object that's probably OK. So as long as it was
> > intentional, I won't object to it.
>
> Also, I wonder how friendly this would be on RISC targets. The use of
> simplify_gen_unary...
>
> > + /* If we have (set (subreg:M (reg:N DEST) SRC) with M wider than N,
> > + we have a paradoxical subreg destination, such as created by the
> > + clause above, check if we can simplify (subreg:N SRC) to eliminate
> > + the (paradoxical) subreg. */
> > + else if (GET_CODE (dest) == SUBREG
> > + && subreg_lowpart_p (dest)
> > + && paradoxical_subreg_p (dest)
> > + && SCALAR_INT_MODE_P (mode)
> > + && SCALAR_INT_MODE_P (GET_MODE (SUBREG_REG (dest))))
> > + {
> > + rtx tmp = simplify_gen_unary (TRUNCATE, GET_MODE (SUBREG_REG
> (dest)),
> > + src, mode);
>
> ...here means that we'd narrow the operation even if no simplification of the rhs
> is possible. So, for example, I can imagine this trying to create a QImode
> operation on targets that only provide SImode or DImode operations.
>
> This seems like something that would be worth trying if a recog of the original
> pattern fails, but maybe isn't something that we should commit to up-front, before
> any recog has been attempted.
>
> (BTW, the simplify_lowpart_p check is redundant with the paradoxical_subreg_p
> check.)
>
> Thanks,
> Richard
>
> > + if (tmp)
> > + {
> > + SUBST (SET_DEST (x), SUBREG_REG (dest));
> > + SUBST (SET_SRC (x), tmp);
> > + dest = SET_DEST (x);
> > + src = SET_SRC (x);
> > + }
> > + }
> > +
Hi Jeff, Thanks for the review. Jeff Law wrote: > On 1/8/2026 7:30 PM, Roger Sayle wrote: > > This patch addresses a code quality regression on x86_64 related to PR > > 123236. That original PR (and the related PR 101266) concern tree > > level optimizations, where this problem should also be fixed, but it > > also reveals a regression in the RTL optimizers. > > > > A motivating test case (on x86_64) is: > > > > unsigned int foo(unsigned int a) { > > unsigned long long t = a; > > return t >> 4; > > } > > > > which with -O2 currently generates (2 64-bit shifts): > > > > foo: movq %rdi, %rax > > salq $32, %rax > > shrq $36, %rax > > ret > > > > with this patch, we now generate (1 32-bit shift): > > > > foo: movl %edi, %eax > > shrl $4, %eax > > ret > > > > which matches what GCC generated prior to GCC 11. > > Likewise, for the signed (SIGN_EXTRACT/SIGN_EXTEND) case: > > > > int bar(int a) { > > long long t = a; > > return t >> 4; > > } > > > > Before: > > bar: movslq %edi, %rax > > sarq $4, %rax > > ret > > > > After: > > bar: movl %edi, %eax > > sarl $4, %eax > > ret > > > > The underlying cause of the RTL-level regression is that some RTL > > expressions that were previously expressed as {ZERO,SIGN}_EXTEND are > > now sometimes represented as the equivalent {ZERO,SIGN}_EXTRACT, and > > that not all simplifications of TRUNCATE({ZERO,SIGN}_EXTEND) are > > implemented for TRUNCATE({ZERO,SIGN}_EXTRACT). > > > > Thanks to Segher, simplify_rtx does handle some truncations of > > extracts, see > > https://gcc.gnu.org/pipermail/gcc-patches/2016-November/463629.html > > but unfortunately this code (and subsequent tweaks) doesn't quite > > match the cases that we care about. > > > > Previously: > > Trying 6, 7 -> 8: > > 6: r103:DI=sign_extend(r105:SI) > > REG_DEAD r105:SI > > 7: {r104:DI=r103:DI>>0x4;clobber flags:CC;} > > REG_DEAD r103:DI > > REG_UNUSED flags:CC > > 8: r102:SI=r104:DI#0 > > REG_DEAD r104:DI > > Failed to match this instruction: > > (set (subreg:DI (reg:SI 102 [ _4 ]) 0) > > (zero_extend:DI (subreg:SI (sign_extract:DI (reg:SI 105 [ aD.2962 ]) > > (const_int 28 [0x1c]) > > (const_int 4 [0x4])) 0))) > > > > With the tweaks below, this becomes: > > Trying 6, 7 -> 8: > > 6: r103:DI=sign_extend(r105:SI) > > REG_DEAD r105:SI > > 7: {r104:DI=r103:DI>>0x4;clobber flags:CC;} > > REG_DEAD r103:DI > > REG_UNUSED flags:CC > > 8: r102:SI=r104:DI#0 > > REG_DEAD r104:DI > > Successfully matched this instruction: > > (set (reg:SI 102 [ _4 ]) > > (ashiftrt:SI (reg:SI 105 [ aD.2962 ]) > > (const_int 4 [0x4]))) > > allowing combination of insns 6, 7 and 8 original costs 4 + 4 + 4 = 12 > > replacement cost 4 deferring deletion of insn with uid = 7. > > deferring deletion of insn with uid = 6. > > modifying insn i3 8: {r102:SI=r105:SI>>0x4;clobber flags:CC;} > > REG_UNUSED flags:CC > > REG_DEAD r105:SI > > deferring rescan insn with uid = 8. > > > > This requires two minor tweaks. The first is that the paradoxical > > subreg assignment (set (subreg:DI (reg:SI 102) 0) (expr:DI)) can > > (sometimes) be simplified to (set (reg:SI 102) (truncate:SI (expr:DI)) > > especially if the (truncate:SI (expr:DI)) can itself be simplified. > > The second is that (truncate:SI (sign_extract:DI (reg:SI 102) x y)) > > slips through the existing simplify_truncation transformations as the > > the {zero,sign}_extract has a different mode to its first operand. > > > > 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? > > > > 2026-01-09 Roger Sayle <roger@nextmovesoftware.com> > > > > gcc/ChangeLog > > PR rtl-optimization/123236 > > * combine.cc (simplify_set): Attempt to simplify SETs where the > > destination is a low-part paradoxical subreg between scalar integer > > modes. > > * simplify-rtx.cc (simplify_context::simplify_truncation): Handle > > cases where a ZERO_EXTRACT or SIGN_EXTRACT has a different mode > > to (at least as wide as) its first operand. > > > > gcc/testsuite/ChangeLog > > PR rtl-optimization/123236 > > * gcc.target/i386/pr123236.c: New test case. > So in simplify_truncation you dropped the REG_P (XEXP (op, 0)) check. Was that > intentional? I guess since you don't ultimately change the mode of the truncated > object that's probably OK. So as long as it was intentional, I won't object to it. > > So if the removal was intentional, then OK as-is. If it was a goof, then OK with the > check restored. Yes, removing this check was intentional. As you know, combine smashes together instructions and relies on simplify-rtx to clean up the resulting "fusion products" into something reasonable. So in a machine description, the first operand of a ????_extract is expected to be a register, but during combine it could be anything. The correctness of this transformation isn’t dependent upon where XEXP (op, 0) is a register or not, so restricting it just leads to missed optimization opportunities. I must admit that when removing this test I was thinking about the above RTL: > > (set (subreg:DI (reg:SI 102 [ _4 ]) 0) > > (zero_extend:DI (subreg:SI (sign_extract:DI (reg:SI 105 [ aD.2962 ]) > > (const_int 28 [0x1c]) > > (const_int 4 [0x4])) 0))) Here the first operand of a subreg should almost always be a REG (or MEM), but clearly this is a case where simplify_rtx needs to be able handle something more exotic. Indeed a major motivation for this patch is to avoid creating that very dubious SUBREG in the first place. To address Richard Sandiford's concerns, I'd hope there a very few places in RISC backends, where a (middle-end generated) SUBREG of a non-REG/non-MEM rtx was recognized as a valid instruction. Having simplify-rtx generate a pattern that could potentially be recognized (and costed) can only improve code quality. Along a similar line, I had a related old patch from November 2023, PR112380: Defend against CLOBBERs in RTX expressions in combine.cc https://gcc.gnu.org/pipermail/gcc-patches/2023-November/636203.html that likewise realized that returning a rtx containing (clobber (const_int 0)) is never going to be recognized (should never be recognized), so having simplify-rtx notice this and try something else, can also only improve code quality. I'm happy to wait a little while to investigate usage of paradoxical SUBREG SETs on aarch64 and other RISC architectures, especially if someone can come up with a concrete example of things getting worse, rather than just speculation. The patch can always be reverted if we encounter cases I've not considered, or there's a proposal to fix this in combine/simplify-rtx somewhere else that addresses the regression on x86_64. Cheers, Roger --
On 5/4/2026 5:32 PM, Roger Sayle wrote: >>> (set (subreg:DI (reg:SI 102 [ _4 ]) 0) >>> (zero_extend:DI (subreg:SI (sign_extract:DI (reg:SI 105 [ aD.2962 ]) >>> (const_int 28 [0x1c]) >>> (const_int 4 [0x4])) 0))) > Here the first operand of a subreg should almost always be a REG (or MEM), but > clearly this is a case where simplify_rtx needs to be able handle something more > exotic. Indeed a major motivation for this patch is to avoid creating that very > dubious SUBREG in the first place. > > To address Richard Sandiford's concerns, I'd hope there a very few places in RISC > backends, where a (middle-end generated) SUBREG of a non-REG/non-MEM rtx > was recognized as a valid instruction. Having simplify-rtx generate a pattern that > could potentially be recognized (and costed) can only improve code quality. So I looked fairly seriously at using SUBREGs of things like basic arithmetic and shifts to express that there are times where we don't care about the upper bits, but for "reasons" we can't change the mode. If we do this before combine it opens up more possibilities to use the single bit manipulation instructions for RISC-V. In fact, I suspect it's the single largest source of missed bset/bclr/binv opportunities for rv64. I've got prototype code lying around here; I'm not at all happy with it right now and it's not in use anywhere. Philipp and his team are approaching the problem differently by promoting the operations to DI when safe. That may be sufficient as well, I need to sit down and play with it for a while. > I'm happy to wait a little while to investigate usage of paradoxical SUBREG SETs > on aarch64 and other RISC architectures, especially if someone can come up > with a concrete example of things getting worse, rather than just speculation. > The patch can always be reverted if we encounter cases I've not considered, > or there's a proposal to fix this in combine/simplify-rtx somewhere else that > addresses the regression on x86_64. I don't have anything concrete that would be negatively affected by your change at this time. jeff
[Combining replies because I'm only really making a single point] "Roger Sayle" <roger@nextmovesoftware.com> writes: > The same class of bug is also present on aarch64, as an example of a RISC > target, but there it presents differently in the RTL, due to the internal > representations making less use of SUBREG. > > Consider the four functions: > > int sfoo(int a) { > long long t = a; > return t >> 4; > } > > unsigned int ufoo(unsigned int a) { > unsigned long long t = a; > return t >> 4; > } > > long long sbar(long long x) > { > __int128 t = x; > return t >> 4; > } > > unsigned long long ubar(unsigned long long x) > { > unsigned __int128 t = x; > return t >> 4; > } > > With -O2, GCC on aarch64 currently generates: > > sfoo: > sbfx x0, x0, 4, 28 > ret > ufoo: > lsr w0, w0, 4 > ret > sbar: > asr x1, x0, 63 > extr x0, x1, x0, 4 > ret > ubar: > lsr x0, x0, 4 > ret > > Whereas current clang/LLVM generates (the hoped for): > > sfoo: > asr w0, w0, #4 > ret > > ufoo: > lsr w0, w0, #4 > ret > > sbar: > asr x0, x0, #4 > ret > > ubar: > lsr x0, x0, #4 > ret > > > So the motivation for the patch is independent of the target, > it's just that some (RISC) backends don’t suffer SUBREG headaches > to the same extent as others (such as x86 and AVR). I'm not surprised that you've found an aarch64 case that benefits from the patch. To be clear, I wasn't claiming otherwise. My concerns were: (1) As far as I can tell, all the motivating examples in the discussion have involved cases where applying simplify_gen_unary to the src produces a simplification. But simplify_gen_unary will produce a new expression even if no simplification is possible. Thus if we have: (set (subreg:DI (reg:SI X) 0) (lshiftrt:DI ....)) the patch (AFAICT) will produce: (set (reg:SI X) (subreg:SI (lshiftrt:DI ...) 0)) if the shift cannot be simplified. (2) All the motivating examples have involved cases where the inner register of the subreg is a native arithmetic mode for the target. But the transformation applies to other modes too, such as QImode and HImode on RISCy targets. "Roger Sayle" <roger@nextmovesoftware.com> writes: > I must admit that when removing this test I was thinking about the above RTL: >> > (set (subreg:DI (reg:SI 102 [ _4 ]) 0) >> > (zero_extend:DI (subreg:SI (sign_extract:DI (reg:SI 105 [ aD.2962 ]) >> > (const_int 28 [0x1c]) >> > (const_int 4 [0x4])) 0))) > > Here the first operand of a subreg should almost always be a REG (or > MEM), but clearly this is a case where simplify_rtx needs to be able > handle something more exotic. Indeed a major motivation for this > patch is to avoid creating that very dubious SUBREG in the first > place. > > To address Richard Sandiford's concerns, I'd hope there a very few > places in RISC backends, where a (middle-end generated) SUBREG of a > non-REG/non-MEM rtx was recognized as a valid instruction. Right. But that's my point :) If no simplification is possible, simplify_gen_unary will wrap the original SET_SRC in a subreg or a truncate (depending on TRULY_NOOP_TRUNCATION). That isn't likely to match if the SET_SRC is anything other than a trivial operand. Even if the simplify_gen_unary does simplify, there are cases where that might not be what we want. On a 32-bit WORD_REGISTER_OPERATIONS target, it seems wrong to try to simplify (say) a 32-bit addition to a 16-bit addition just because the target is (subreg:SI (reg:HI ...) 0). (FTR, aarch64 is not a WORD_REGISTER_OPERATIONS target, which IMO is a good thing :) But many other targets are, of course.) The crux of my objection is this: it means that, if we have one i3 of the form: A: (set (reg:SI RA) src) and another i3 of the form: B: (set (subreg:SI (reg:HI RB) 0) src) with identical src, and with identical instructions feeding src, combine will now tend to succeed for A on RISCy targets in ways in which it wouldn't succeed for B, even though both destinations match register_operand. That isn't something that I would have expected from combine until now. Like I say, I can see why this transformation is attractive. It just feels like something that would be better done at the recog stage. My original suggestion was to try the transformation if the initial recog fails, but I suppose that doesn't help with your use case. An alternative would be to try matching both forms. If only one matches, keep that. If both match, keep the cheapest. If they're equally cheap, keep the one with the simple destination. > Having simplify-rtx generate a pattern that could potentially be > recognized (and costed) can only improve code quality. > > [...] > I'm happy to wait a little while to investigate usage of paradoxical > SUBREG SETs on aarch64 and other RISC architectures, especially if > someone can come up with a concrete example of things getting worse, > rather than just speculation. The patch can always be reverted if we > encounter cases I've not considered, or there's a proposal to fix this > in combine/simplify-rtx somewhere else that addresses the regression > on x86_64. Fair enough. I'm really just restating my original objection, rather than adding anything new, so I won't push it further. Richard
diff --git a/gcc/combine.cc b/gcc/combine.cc index 66963222efc..b0c930ae5f5 100644 --- a/gcc/combine.cc +++ b/gcc/combine.cc @@ -7066,6 +7066,27 @@ simplify_set (rtx x) src = SET_SRC (x), dest = SET_DEST (x); } + /* If we have (set (subreg:M (reg:N DEST) SRC) with M wider than N, + we have a paradoxical subreg destination, such as created by the + clause above, check if we can simplify (subreg:N SRC) to eliminate + the (paradoxical) subreg. */ + else if (GET_CODE (dest) == SUBREG + && subreg_lowpart_p (dest) + && paradoxical_subreg_p (dest) + && SCALAR_INT_MODE_P (mode) + && SCALAR_INT_MODE_P (GET_MODE (SUBREG_REG (dest)))) + { + rtx tmp = simplify_gen_unary (TRUNCATE, GET_MODE (SUBREG_REG (dest)), + src, mode); + if (tmp) + { + SUBST (SET_DEST (x), SUBREG_REG (dest)); + SUBST (SET_SRC (x), tmp); + dest = SET_DEST (x); + src = SET_SRC (x); + } + } + /* If we have (set FOO (subreg:M (mem:N BAR) 0)) with M wider than N, this would require a paradoxical subreg. Replace the subreg with a zero_extend to avoid the reload that would otherwise be required. diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc index 8016e02e925..e27ca8be5eb 100644 --- a/gcc/simplify-rtx.cc +++ b/gcc/simplify-rtx.cc @@ -727,12 +727,10 @@ simplify_context::simplify_truncation (machine_mode mode, rtx op, } } - /* Turn (truncate:M1 (*_extract:M2 (reg:M2) (len) (pos))) into - (*_extract:M1 (truncate:M1 (reg:M2)) (len) (pos')) if possible without - changing len. */ + /* Turn (truncate:M1 (*_extract:M2 (reg:M3) (len) (pos))) into + (*_extract:M1 (truncate:M1 (reg:M3)) (len) (pos')) if possible. */ if ((GET_CODE (op) == ZERO_EXTRACT || GET_CODE (op) == SIGN_EXTRACT) - && REG_P (XEXP (op, 0)) - && GET_MODE (XEXP (op, 0)) == GET_MODE (op) + && precision <= GET_MODE_UNIT_PRECISION (GET_MODE (XEXP (op, 0))) && CONST_INT_P (XEXP (op, 1)) && CONST_INT_P (XEXP (op, 2))) { @@ -741,7 +739,8 @@ simplify_context::simplify_truncation (machine_mode mode, rtx op, unsigned HOST_WIDE_INT pos = UINTVAL (XEXP (op, 2)); if (BITS_BIG_ENDIAN && pos >= op_precision - precision) { - op0 = simplify_gen_unary (TRUNCATE, mode, op0, GET_MODE (op0)); + if (GET_MODE (op0) != mode) + op0 = simplify_gen_unary (TRUNCATE, mode, op0, GET_MODE (op0)); if (op0) { pos -= op_precision - precision; @@ -751,7 +750,8 @@ simplify_context::simplify_truncation (machine_mode mode, rtx op, } else if (!BITS_BIG_ENDIAN && precision >= len + pos) { - op0 = simplify_gen_unary (TRUNCATE, mode, op0, GET_MODE (op0)); + if (GET_MODE (op0) != mode) + op0 = simplify_gen_unary (TRUNCATE, mode, op0, GET_MODE (op0)); if (op0) return simplify_gen_ternary (GET_CODE (op), mode, mode, op0, XEXP (op, 1), XEXP (op, 2)); diff --git a/gcc/testsuite/gcc.target/i386/pr123236.c b/gcc/testsuite/gcc.target/i386/pr123236.c new file mode 100644 index 00000000000..b369b6fd299 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr123236.c @@ -0,0 +1,18 @@ +/* { dg-do compile { target { ! ia32 } } } */ +/* { dg-options "-O2" } */ + +unsigned int foo(unsigned int a) { + unsigned long long t = a; + return t >> 4; +} + +int bar(int a) { + long long t = a; + return t >> 4; +} + +/* { dg-final { scan-assembler-not "movq" } } */ +/* { dg-final { scan-assembler-not "salq" } } */ +/* { dg-final { scan-assembler-not "shrq" } } */ +/* { dg-final { scan-assembler-not "movslq" } } */ +/* { dg-final { scan-assembler-not "sarq" } } */