From patchwork Sun Aug 4 19:12:41 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Law X-Patchwork-Id: 1968844 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20230601 header.b=eYouMy/S; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces~incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WcTk14kTfz1yY3 for ; Mon, 5 Aug 2024 05:13:13 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 25FCC3858C32 for ; Sun, 4 Aug 2024 19:13:09 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-pl1-x62b.google.com (mail-pl1-x62b.google.com [IPv6:2607:f8b0:4864:20::62b]) by sourceware.org (Postfix) with ESMTPS id 0FF453858D26 for ; Sun, 4 Aug 2024 19:12:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0FF453858D26 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 0FF453858D26 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::62b ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1722798770; cv=none; b=SWQV4pPWbF7lI/kwdkfEEsHDzQhSoMnxKZt9JICNGQPg9BLQNp+t53A2tQXOoN6bGdibgxOP6tlQGyYSBl/hp4+Huyu74LeUhSEGCorBR5M9MEYE9CFhveLhZ6roJI6ZjbvACReWF9racFKSdw1HB3URIiCgAW2jhlPuAyiEWmI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1722798770; c=relaxed/simple; bh=vrvnQqSBCFSGTdCvpFy2MimRy0DAfhmgCzZsT++KKEE=; h=DKIM-Signature:Message-ID:Date:MIME-Version:From:To:Subject; b=fSqq57+w8UEvWpDWitst6iPHglvdkw6iSESK6tOzjsuJXqL7RFzdHLUN4NXCtr2O4RRERuLgViv8NVnkryvuzjuviSLIVGNYaWCWLBjma7m1VxhouQnD3APvQfv0Mgsu7BFZVvoqiVVcT+qsJ/CGS9NdX97As8Vmlla2VP8iS1s= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pl1-x62b.google.com with SMTP id d9443c01a7336-1fc491f9b55so76745095ad.3 for ; Sun, 04 Aug 2024 12:12:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1722798766; x=1723403566; darn=gcc.gnu.org; h=subject:to:from:content-language:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=TPzuncdgLVNkvCV6Z7OcjRTxktyOKYBNSDXHRwAXfPs=; b=eYouMy/S1P16mCbPpFX5ueomBN+n4JUIjC27aBWJTXOZvNlLJz5p7aG0Ph1juwbDhk 4T1WkXJ3Gh/XbOz6U1WzxB+ngpsU1YT7MCZn8rntMg/l/QK8QnXDdyRgCMiFqpHF/8XX xjCIPHXEa5Vm1h+cjTC89vXuAAPnLNtvl9IEWFoXQRlXxVFjJv5JQUJxRdT2w20cCioz FWgDL/Zl7foo4tCKbFDEIWXTUlAeAzSKoSSN6yr36VEOXH9riXIAjpQCva7kPifmqnzo OzSbRbIJd+mIavDXYGJ+uhgeEJ6vyjB71chXJOUCD3l7ipQ3mzr1e4CTS4UA4hqJ8KOe byQQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722798766; x=1723403566; h=subject:to:from:content-language:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=TPzuncdgLVNkvCV6Z7OcjRTxktyOKYBNSDXHRwAXfPs=; b=ajFlPqa1+9JI/gvBy0swe8HAYdm5sVCBINOKt0MFhFzL6Z8pkm9gTXK/0pe9sfpsKJ HLDG4Kb2R2/O8SyLRFPGY8bat96nsKAgWs+NFxPkDOiamQ2G4olgZ/0Flo5yGDEIGItA WgYCqc20Rd/ynL5pzKxhChiCWrBuA+3/IyteJDSGRB2kYecTXo/XgZbfyd97MEgCgObV afVggxe+8Jx+K0/WjbBZxS2NPi3B0bxoiWbjmHfmdRzgCDSMuPvbyaKNHLNiITptTQDJ K5mLzFysLhPYPlDLaX0UfzGbQKSkiadzIatDwk+FJgYYqh+YDXRUIy1m/VUSD4Y9NCzr oj0Q== X-Gm-Message-State: AOJu0YwboXPt7zqJ9iPg5ui/0ZAcMwLD9VoPbNoiOufQW4Mc6jYMr25n JXfH/M85CsWRX9gkYiWDzOc6Cz/rodSDfLi2OnIDhAxXQw+TArEzlHNwtdYC X-Google-Smtp-Source: AGHT+IHRCUYuxQAsW2TB7f7dtJUGhoQ2mNbEIUNtQ3u76X/X9NOFZijv9DZNRyNnYAHQEc8lu1wePw== X-Received: by 2002:a17:902:f94e:b0:1fd:70c4:8389 with SMTP id d9443c01a7336-1ff5722e2f9mr68038465ad.7.1722798766342; Sun, 04 Aug 2024 12:12:46 -0700 (PDT) Received: from [172.31.0.109] ([136.36.72.243]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1ff58f57624sm54722475ad.98.2024.08.04.12.12.44 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 04 Aug 2024 12:12:45 -0700 (PDT) Message-ID: <3329c491-59e9-4d18-b1ba-901610e50c94@gmail.com> Date: Sun, 4 Aug 2024 13:12:41 -0600 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Content-Language: en-US From: Jeff Law To: "gcc-patches@gcc.gnu.org" Subject: [committed][PR rtl-optimization/116199] Fix latent bug in reload's SUBREG handling X-Spam-Status: No, score=-8.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, WEIRD_PORT autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces~incoming=patchwork.ozlabs.org@gcc.gnu.org Building glibc on the m68k has exposed a long standing latent bug in reload. Basically ext-dce replaced an extension with a subreg expression (good) resulting in this pair of insns: > (insn 7 4 8 2 (set (reg:DI 31 [ _1 ]) > (subreg:DI (reg/v:SI 37 [ __major ]) 0)) "j.c":7:32 75 {*m68k.md:1568} > (nil)) > (insn 8 7 10 2 (set (reg:DI 39 [ _2 ]) > (ashift:DI (reg:DI 31 [ _1 ]) > (const_int 8 [0x8]))) "j.c":7:48 322 {*ashldi3} > (expr_list:REG_DEAD (reg:DI 31 [ _1 ]) > (nil))) insn 7 was optimized to the simple copy by ext-dce. That looks fine. Combine comes along and squashes them together resulting in: > (insn 8 7 10 2 (set (reg:DI 39 [ _2 ]) > (ashift:DI (subreg:DI (reg/v:SI 37 [ __major ]) 0) > (const_int 8 [0x8]))) "j.c":7:48 322 {*ashldi3} > (nil)) Which also looks good. After IRA's allocation, in the middle of reload we have: > (insn 8 7 10 2 (set (reg:DI 8 %a0 [orig:39 _2 ] [39]) > (ashift:DI (subreg:DI (reg/v:SI 0 %d0 [orig:37 __major ] [37]) 0) > (const_int 8 [0x8]))) "j.c":7:48 322 {*ashldi3} > (nil)) Again, sensible. The pattern requires op0 and op1 to match, so we try to figure out if d0 & a0 are the same underlying register. So we get into this code in operands_match_p: > if (code == SUBREG) > { > i = REGNO (SUBREG_REG (x)); > if (i >= FIRST_PSEUDO_REGISTER) > goto slow; > i += subreg_regno_offset (REGNO (SUBREG_REG (x)), > GET_MODE (SUBREG_REG (x)), > SUBREG_BYTE (x), > GET_MODE (x)); > } > else > i = REGNO (x); There's a similar fragment for the other operand. The key is that subreg_regno_offset call. That call assumes the subreg is representable. But in the case of (subreg:DI (reg:SI d0)) we're going to get -1 (remember, m68k is a big endian target). That -1 gets passed to hard_regno_regs via this code (again, just showing one of the two copies of this fragment): > if (REG_WORDS_BIG_ENDIAN > && is_a (GET_MODE (x), &xmode) > && GET_MODE_SIZE (xmode) > UNITS_PER_WORD > && i < FIRST_PSEUDO_REGISTER) > i += hard_regno_nregs (i, xmode) - 1; That triggers the reported ICE. It appears this has been broken since the conversion to SUBREG_BYTE way back in 2001, though possibly it could have been some minor changes around this code circa 2005 as well, it didn't seem worth putting under the debugger to be sure. Certainly the code from 2001 looks suspicious to me. Anyway, the fix here is pretty simple. The routine "simplify_subreg_regno" is meant to be used to determine if we can simplify the subreg expression and will explicitly return -1 if it can't be represented for one reason or another. It checks a variety of conditions that aren't worth listing here. Bootstrapped and regression tested on x86 (after reverting an unrelated patch from Richard S that's causing multiple unrelated failures), which of course doesn't really test the code as x86 is an LRA target. Also built & tested the crosses, none of which show issues (and some of which are reload targets). m68k will bootstrap & regression test tomorrow, but I don't think there's any point in waiting for that. Pushing to the trunk. jeff commit cfeb994d64743691d4a787f8eef7ce52d8711756 Author: Jeff Law Date: Sun Aug 4 13:09:02 2024 -0600 [committed][PR rtl-optimization/116199] Fix latent bug in reload's SUBREG handling Building glibc on the m68k has exposed a long standing latent bug in reload. Basically ext-dce replaced an extension with a subreg expression (good) resulting in this pair of insns: > (insn 7 4 8 2 (set (reg:DI 31 [ _1 ]) > (subreg:DI (reg/v:SI 37 [ __major ]) 0)) "j.c":7:32 75 {*m68k.md:1568} > (nil)) > (insn 8 7 10 2 (set (reg:DI 39 [ _2 ]) > (ashift:DI (reg:DI 31 [ _1 ]) > (const_int 8 [0x8]))) "j.c":7:48 322 {*ashldi3} > (expr_list:REG_DEAD (reg:DI 31 [ _1 ]) > (nil))) insn 7 was optimized to the simple copy by ext-dce. That looks fine. Combine comes along and squashes them together resulting in: > (insn 8 7 10 2 (set (reg:DI 39 [ _2 ]) > (ashift:DI (subreg:DI (reg/v:SI 37 [ __major ]) 0) > (const_int 8 [0x8]))) "j.c":7:48 322 {*ashldi3} > (nil)) Which also looks good. After IRA's allocation, in the middle of reload we have: > (insn 8 7 10 2 (set (reg:DI 8 %a0 [orig:39 _2 ] [39]) > (ashift:DI (subreg:DI (reg/v:SI 0 %d0 [orig:37 __major ] [37]) 0) > (const_int 8 [0x8]))) "j.c":7:48 322 {*ashldi3} > (nil)) Again, sensible. The pattern requires op0 and op1 to match, so we try to figure out if d0 & a0 are the same underlying register. So we get into this code in operands_match_p: > if (code == SUBREG) > { > i = REGNO (SUBREG_REG (x)); > if (i >= FIRST_PSEUDO_REGISTER) > goto slow; > i += subreg_regno_offset (REGNO (SUBREG_REG (x)), > GET_MODE (SUBREG_REG (x)), > SUBREG_BYTE (x), > GET_MODE (x)); > } > else > i = REGNO (x); There's a similar fragment for the other operand. The key is that subreg_regno_offset call. That call assumes the subreg is representable. But in the case of (subreg:DI (reg:SI d0)) we're going to get -1 (remember, m68k is a big endian target). That -1 gets passed to hard_regno_regs via this code (again, just showing one of the two copies of this fragment): > if (REG_WORDS_BIG_ENDIAN > && is_a (GET_MODE (x), &xmode) > && GET_MODE_SIZE (xmode) > UNITS_PER_WORD > && i < FIRST_PSEUDO_REGISTER) > i += hard_regno_nregs (i, xmode) - 1; That triggers the reported ICE. It appears this has been broken since the conversion to SUBREG_BYTE way back in 2001, though possibly it could have been some minor changes around this code circa 2005 as well, it didn't seem worth putting under the debugger to be sure. Certainly the code from 2001 looks suspicious to me. Anyway, the fix here is pretty simple. The routine "simplify_subreg_regno" is meant to be used to determine if we can simplify the subreg expression and will explicitly return -1 if it can't be represented for one reason or another. It checks a variety of conditions that aren't worth listing here. Bootstrapped and regression tested on x86 (after reverting an unrelated patch from Richard S that's causing multiple unrelated failures), which of course doesn't really test the code as x86 is an LRA target. Also built & tested the crosses, none of which show issues (and some of which are reload targets). m68k will bootstrap & regression test tomorrow, but I don't think there's any point in waiting for that. Pushing to the trunk. PR rtl-optimization/116199 gcc/ * reload.cc (operands_match_p): Verify subreg is expressable before trying to simplify and match it to another operand. gcc/testsuite/ * gcc.dg/torture/pr116199.c: New test. diff --git a/gcc/reload.cc b/gcc/reload.cc index fecf245c827..f849f1559dc 100644 --- a/gcc/reload.cc +++ b/gcc/reload.cc @@ -2211,7 +2211,11 @@ operands_match_p (rtx x, rtx y) if (code == SUBREG) { i = REGNO (SUBREG_REG (x)); - if (i >= FIRST_PSEUDO_REGISTER) + if (i >= FIRST_PSEUDO_REGISTER + || simplify_subreg_regno (REGNO (SUBREG_REG (x)), + GET_MODE (SUBREG_REG (x)), + SUBREG_BYTE (x), + GET_MODE (x)) == -1) goto slow; i += subreg_regno_offset (REGNO (SUBREG_REG (x)), GET_MODE (SUBREG_REG (x)), @@ -2224,7 +2228,11 @@ operands_match_p (rtx x, rtx y) if (GET_CODE (y) == SUBREG) { j = REGNO (SUBREG_REG (y)); - if (j >= FIRST_PSEUDO_REGISTER) + if (j >= FIRST_PSEUDO_REGISTER + || simplify_subreg_regno (REGNO (SUBREG_REG (y)), + GET_MODE (SUBREG_REG (y)), + SUBREG_BYTE (y), + GET_MODE (y)) == -1) goto slow; j += subreg_regno_offset (REGNO (SUBREG_REG (y)), GET_MODE (SUBREG_REG (y)), diff --git a/gcc/testsuite/gcc.dg/torture/pr116199.c b/gcc/testsuite/gcc.dg/torture/pr116199.c new file mode 100644 index 00000000000..1012816180e --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr116199.c @@ -0,0 +1,13 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target int32 } */ + +__extension__ typedef unsigned long long int __uint64_t; +__extension__ typedef __uint64_t __dev_t; + +__dev_t __gnu_dev_makedev (unsigned int __major, unsigned int __minor) +{ + __dev_t __dev; + __dev = (((__dev_t) (__major & 0x00000fffu)) << 8); + __dev |= (((__dev_t) (__major & 0xfffff000u)) << 32); + return __dev; +}