From patchwork Thu Aug 15 08:45:36 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 1972698 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org 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 4WkzHc72TXz1yNr for ; Thu, 15 Aug 2024 18:46:20 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id CF0423858289 for ; Thu, 15 Aug 2024 08:46:18 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 665A23858404 for ; Thu, 15 Aug 2024 08:45:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 665A23858404 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 665A23858404 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=217.140.110.172 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1723711540; cv=none; b=tMoKB+a2DmvRGR/De0ZVsxWVJeX67innxdVwol5gqYDoe7mSRTjV1vhVGwuaHckxoqmYQ64UL8+/QgmI4WW4i8QBWxa/ziHvqPmUTw4W3Ir97AgFwQrQ5g0oXIxhKzTsSyWSeQJEvu0LmXVhPKYRUI6jIsMuYGIrtGVYx6Z59C4= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1723711540; c=relaxed/simple; bh=/7syFICuqVmaaf2ePEh+SHTVXe1uTzNw4Kdstu75uyI=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=BmP+iY2ZS7XNXGFX7mPkBEqWHunHYs6NHYfIJKWFJ8bDjAgJHE6yHVEAVVJQOEkfJKjOLaWSTmwfaMsC39xIFR48vBVYq5T5vZWcweCPl0WnhySH+cfjDg6JRV2Y4kSY/YAILclS6BpUiaUNuiR/KVGYGtIY7hVNWXYjz6eiJGE= ARC-Authentication-Results: i=1; server2.sourceware.org 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 04C7B169E for ; Thu, 15 Aug 2024 01:46:04 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.110.72]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A8CEA3F6A8 for ; Thu, 15 Aug 2024 01:45:37 -0700 (PDT) From: Richard Sandiford To: gcc-patches@gcc.gnu.org Mail-Followup-To: gcc-patches@gcc.gnu.org, richard.sandiford@arm.com Subject: [PATCH] late-combine: Preserve INSN_CODE when modifying notes [PR116343] Date: Thu, 15 Aug 2024 09:45:36 +0100 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 X-Spam-Status: No, score=-19.0 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE 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 When it removes a definition, late-combine tries to update all uses in notes. It does this using the same insn_propagation class that it uses for patterns. However, insn_propagation uses validate_change, which in turn resets the INSN_CODE. This is inefficient in the best case, since it forces the pattern to be rerecognised even though changing a note can't affect the INSN_CODE. But in the PR it's a correctness problem: resetting INSN_CODE means we lose the NOOP_INSN_MOVE_CODE, which in turn means that rtl-ssa doesn't queue it for deletion. This patch adds a routine specifically for propagating into notes. A belt-and-braces fix would be to rerecognise noop moves in function_info::change_insns, but I can't think of a good reason why that would be necessary, and it could paper over latent bugs. Tested on aarch64-linux-gnu & x86_64-linux-gnu. OK to install? Richard gcc/ PR testsuite/116343 * recog.h (insn_propagation::apply_to_note): Declare. * recog.cc (insn_propagation::apply_to_note): New function. * late-combine.cc (insn_combination::substitute_note): Use apply_to_note instead of apply_to_rvalue. * rtl-ssa/changes.cc (rtl_ssa::changes_are_worthwhile): Improve dumping of costs for noop moves. gcc/testsuite/ PR testsuite/116343 * gcc.dg/torture/pr116343.c: New test. --- gcc/late-combine.cc | 2 +- gcc/recog.cc | 13 +++++++++++++ gcc/recog.h | 1 + gcc/rtl-ssa/changes.cc | 5 ++++- gcc/testsuite/gcc.dg/torture/pr116343.c | 18 ++++++++++++++++++ 5 files changed, 37 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/torture/pr116343.c diff --git a/gcc/late-combine.cc b/gcc/late-combine.cc index 2b62e2956ed..1d81b386c3d 100644 --- a/gcc/late-combine.cc +++ b/gcc/late-combine.cc @@ -338,7 +338,7 @@ insn_combination::substitute_note (insn_info *use_insn, rtx note, || REG_NOTE_KIND (note) == REG_EQUIV) { insn_propagation prop (use_insn->rtl (), m_dest, m_src); - return (prop.apply_to_rvalue (&XEXP (note, 0)) + return (prop.apply_to_note (&XEXP (note, 0)) && (can_propagate || prop.num_replacements == 0)); } return true; diff --git a/gcc/recog.cc b/gcc/recog.cc index 23e4820180f..615aaabc551 100644 --- a/gcc/recog.cc +++ b/gcc/recog.cc @@ -1469,6 +1469,19 @@ insn_propagation::apply_to_rvalue (rtx *loc) return res; } +/* Like apply_to_rvalue, but specifically for the case where *LOC is in + a note. This never changes the INSN_CODE. */ + +bool +insn_propagation::apply_to_note (rtx *loc) +{ + auto old_code = INSN_CODE (insn); + bool res = apply_to_rvalue (loc); + if (INSN_CODE (insn) != old_code) + INSN_CODE (insn) = old_code; + return res; +} + /* Check whether INSN matches a specific alternative of an .md pattern. */ bool diff --git a/gcc/recog.h b/gcc/recog.h index 87a5803dec0..1dccce78ba4 100644 --- a/gcc/recog.h +++ b/gcc/recog.h @@ -121,6 +121,7 @@ public: insn_propagation (rtx_insn *, rtx, rtx, bool = true); bool apply_to_pattern (rtx *); bool apply_to_rvalue (rtx *); + bool apply_to_note (rtx *); /* Return true if we should accept a substitution into the address of memory expression MEM. Undoing changes OLD_NUM_CHANGES and up restores diff --git a/gcc/rtl-ssa/changes.cc b/gcc/rtl-ssa/changes.cc index a30f000191e..0476296607b 100644 --- a/gcc/rtl-ssa/changes.cc +++ b/gcc/rtl-ssa/changes.cc @@ -228,7 +228,10 @@ rtl_ssa::changes_are_worthwhile (array_slice changes, for (const insn_change *change : changes) if (!change->is_deletion ()) { - fprintf (dump_file, " %c %d", sep, change->new_cost); + if (INSN_CODE (change->rtl ()) == NOOP_MOVE_INSN_CODE) + fprintf (dump_file, " %c nop", sep); + else + fprintf (dump_file, " %c %d", sep, change->new_cost); sep = '+'; } if (weighted_new_cost != 0) diff --git a/gcc/testsuite/gcc.dg/torture/pr116343.c b/gcc/testsuite/gcc.dg/torture/pr116343.c new file mode 100644 index 00000000000..ad13f0fc21c --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr116343.c @@ -0,0 +1,18 @@ +// { dg-additional-options "-fschedule-insns -fno-thread-jumps -fno-dce" } + +int a, b, c; +volatile int d; +int e(int f, int g) { return g > 1 ? 1 : f >> g; } +int main() { + int *i = &a; + long j[1]; + if (a) + while (1) { + a ^= 1; + if (*i) + while (1) + ; + b = c && e((d, 1) >= 1, j[0]); + } + return 0; +}