From patchwork Fri Apr 17 21:43:37 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Li, Pan2 via Gcc-patches" X-Patchwork-Id: 1272479 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=gcc.gnu.org Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 493qNg0SWRz9sSd for ; Sat, 18 Apr 2020 07:43:59 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B4EC9385DC08; Fri, 17 Apr 2020 21:43:44 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B4EC9385DC08 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1587159824; bh=vvkwMY6K15wTJkOMp7Vkaju4M416LzLbm9vh3+sZnxs=; h=Subject:To:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=PTRtD3zyedQog0i1Yo35YqrlE+q7wgaP6ghJSO15e3vp4J86RIsmuyx9jaLV0nWJ+ KydgsAR/S911qz9aRXFDfsPbe6ez5ku/EphsdswbXj/PhxL1ZFAEnOVpUWgvjbCPQO 7fYDWj7eWAUkxS2psEg6vC69dd51kDCUI19YLBuE= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by sourceware.org (Postfix) with ESMTP id 4B9EF385DC08 for ; Fri, 17 Apr 2020 21:43:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 4B9EF385DC08 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-338-VKUIYRgoP-yXYGQFkFmmew-1; Fri, 17 Apr 2020 17:43:39 -0400 X-MC-Unique: VKUIYRgoP-yXYGQFkFmmew-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 2598E190D340 for ; Fri, 17 Apr 2020 21:43:38 +0000 (UTC) Received: from ovpn-114-204.phx2.redhat.com (ovpn-114-204.phx2.redhat.com [10.3.114.204]) by smtp.corp.redhat.com (Postfix) with ESMTP id E8C2D5C1C3 for ; Fri, 17 Apr 2020 21:43:37 +0000 (UTC) Message-ID: <396d19a2a5538ec8a420a7f284fe9a87efa48431.camel@redhat.com> Subject: [committed] [PR rtl-optimization/90275] Another 90275 related cse.c fix To: gcc-patches List Date: Fri, 17 Apr 2020 15:43:37 -0600 Organization: Red Hat User-Agent: Evolution 3.34.4 (3.34.4-1.fc31) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-23.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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-Patchwork-Original-From: Jeff Law via Gcc-patches From: "Li, Pan2 via Gcc-patches" Reply-To: law@redhat.com Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" This isn't precisely the same issue that we were originally tracking with 90275, but it's closely related and we might as well stuff it in the same bucket. This time instead of having a NOP copy insn that we can completely ignore and ultimately remove, we have a NOP set within a multi-set PARALLEL. It ultimately triggers the same failure when the source of such a set is a hard register for the same reasons as we've already noted in the BZ and patches-to-date. For prior cases we've been able to mark the insn as a nop set and ignore it for the rest of cse_insn, ultimately removing it. That's not really an option here as there are other sets that we have to preserve. We might be able to fix this instance by splitting the multi-set insn, but I'm not keen to introduce splitting into cse. Furthermore, the target may not be able to split the insn. So I considered this is non-starter. What I finally settled on was to use the existing do_not_record machinery to ignore the nop set within the parallel (and only that set within the parallel). One might argue that we should always ignore a REG_UNUSED set. But I rejected that idea -- we could have cse-able divmod insns where the first had a REG_UNUSED note for a destination, but the second did not. One might also argue that we could have a nop set without a REG_UNUSED in a multi-set parallel and thus we could trigger yet another insert_regs ICE at some point. I tend to think this is a possibility. If we see this happen, we'll have to revisit. One might also argue that cse should, in general, be more tolerant of dead code and nop sets. I'd fully agree with that. But I loathe the idea of revamping that code and fear that we'd likely get it wrong along the way. Certainly not appropriate at the current time. Bootstrapped and regression tested on a variety of *-linux-gnu targets (including arm & aarch64). Also regression tested on all the usual *-elf targets in the tester. Installing on the trunk, jeff commit 3737ccc424c56a2cecff202dd79f88d28850eeb2 Author: Jeff Law Date: Fri Apr 17 15:38:13 2020 -0600 [committed] [PR rtl-optimization/90275] Another 90275 related cse.c fix This time instead of having a NOP copy insn that we can completely ignore and ultimately remove, we have a NOP set within a multi-set PARALLEL. It triggers, the same failure when the source of such a set is a hard register for the same reasons as we've already noted in the BZ and patches-to-date. For prior cases we've been able to mark the insn as a nop set and ignore it for the rest of cse_insn, ultimately removing it. That's not really an option here as there are other sets that we have to preserve. We might be able to fix this instance by splitting the multi-set insn, but I'm not keen to introduce splitting into cse. Furthermore, the target may not be able to split the insn. So I considered this is non-starter. What I finally settled on was to use the existing do_not_record machinery to ignore the nop set within the parallel (and only that set within the parallel). One might argue that we should always ignore a REG_UNUSED set. But I rejected that idea -- we could have cse-able divmod insns where the first had a REG_UNUSED note for a destination, but the second did not. One might also argue that we could have a nop set without a REG_UNUSED in a multi-set parallel and thus we could trigger yet another insert_regs ICE at some point. I tend to think this is a possibility. If we see this happen, we'll have to revisit. PR rtl-optimization/90275 * cse.c (cse_insn): Avoid recording nop sets in multi-set parallels when the destination has a REG_UNUSED note. diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 93badc209ae..47f22327542 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,9 @@ +2020-04-17 Jeff Law + + PR rtl-optimization/90275 + * cse.c (cse_insn): Avoid recording nop sets in multi-set parallels + when the destination has a REG_UNUSED note. + 2020-04-17 Tobias Burnus PR middle-end/94635 diff --git a/gcc/cse.c b/gcc/cse.c index f07bbdbebad..5aaba8d80e0 100644 --- a/gcc/cse.c +++ b/gcc/cse.c @@ -4715,8 +4715,20 @@ cse_insn (rtx_insn *insn) /* Compute SRC's hash code, and also notice if it should not be recorded at all. In that case, - prevent any further processing of this assignment. */ - do_not_record = 0; + prevent any further processing of this assignment. + + We set DO_NOT_RECORD if the destination has a REG_UNUSED note. + This avoids getting the source register into the tables, where it + may be invalidated later (via REG_QTY), then trigger an ICE upon + re-insertion. + + This is only a problem in multi-set insns. If it were a single + set the dead copy would have been removed. If the RHS were anything + but a simple REG, then we won't call insert_regs and thus there's + no potential for triggering the ICE. */ + do_not_record = (REG_P (dest) + && REG_P (src) + && find_reg_note (insn, REG_UNUSED, dest)); hash_arg_in_memory = 0; sets[i].src = src; diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 030550f1661..e5d0d92344c 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2020-04-17 Jeff Law + + PR rtl-optimization/90275 + * gcc.c-torture/compile/pr90275-2.c: New test. + 2020-04-17 Patrick Palka PR c++/94483 diff --git a/gcc/testsuite/gcc.c-torture/compile/pr90275-2.c b/gcc/testsuite/gcc.c-torture/compile/pr90275-2.c new file mode 100644 index 00000000000..9ebf7d9fd1a --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/compile/pr90275-2.c @@ -0,0 +1,12 @@ + +void +a() { + short *b; + short c; + long long *d = a; + for (;;) { + long long *e = a; + (*d *= *e - c) / *b ?: (*b = 0); + } +} +