From patchwork Tue Feb 15 14:44:26 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 51127 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 118EA385803D for ; Tue, 15 Feb 2022 14:44:58 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 118EA385803D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1644936298; bh=WCeKD35DhrwItIRGwoJZ8BNB0qgH5BLYsC5Cgckx2QU=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=ZcaOCD+jkcfMqo2VB0/8EpLGnliZ4hK/MoGZsPQcZHA1rJ5PJpgMXTkgw7kTfJHEY dMVLMOu9ZeXuu7FiceZyhlm690sHf1NDzgznkouHtmBvByRUnFOc4VJRpd8bQGOVMa s1273EEC2It8MN5+cS2O5TSdS9kmSWsHg/QyUsEk= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id 76D983858D20 for ; Tue, 15 Feb 2022 14:44:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 76D983858D20 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 6AD4C1F37B; Tue, 15 Feb 2022 14:44:27 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 4F6D713C9F; Tue, 15 Feb 2022 14:44:27 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id cgQdEku8C2LOLAAAMHmgww (envelope-from ); Tue, 15 Feb 2022 14:44:27 +0000 Date: Tue, 15 Feb 2022 15:44:26 +0100 (CET) To: gcc-patches@gcc.gnu.org Subject: [PATCH] tree-optimization/96881 - CD-DCE and CLOBBERs MIME-Version: 1.0 Message-Id: <20220215144427.4F6D713C9F@imap2.suse-dmz.suse.de> X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Richard Biener via Gcc-patches From: Richard Biener Reply-To: Richard Biener Cc: Jakub Jelinek Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" CD-DCE does not consider CLOBBERs as necessary in the attempt to not prevent DCE of SSA defs it uses. A side-effect of that is that it also removes all its control dependences if they are not made necessary by other means. When we later try to preserve as many CLOBBERs as possible we have to make sure we also preserved the controlling conditions, otherwise a CLOBBER can now appear on a path where it was not executed before, leading to wrong code as seen in the testcase. This removes 20570 clobbers early, and 2642 and 957 in the two late CD-DCE passes during stage2 in gcc/ - note almost all of those are direct clobbers, if only tracking indirect clobbers lazily we remove 39, 12 and 5. It FAILs the stack coalescing test g++.dg/opt/pr80032.C which can be only recovered if we never DCE paths to direct clobbers (and thus those clobbers). The pattern there is if (pred) D.2512 = CLOBBER; else D.2512 = CLOBBER;, basically we have all paths leading to the same clobber but we could safely cut some branches which we do not realize. Handling indirect vs. direct clobbers differently feels wrong, so the option would be to simply stop removing control flow to clobbers. Elsewhere we noticed though that we elide all indirect clobbers only after the last CD-DCE. Removing conditional clobbers too early might also inhibit DSE after inlining, so maybe we should rather keep them and try to optimize things elsewhere? The aggressive clobber removal variant patch also causes In destructor 'auto_timevar::~auto_timevar()', inlined from 'void c_parser_declaration_or_fndef(c_parser*, bool, bool, bool, bool, bool, tree_node**, vec*, bool, tree, oacc_routine_data*, bool*' at /home/rguenther/src/gcc3/gcc/c/c-parser.cc:2569:7: /home/rguenther/src/gcc3/gcc/timevar.h:247:20: error: dangling pointer to 'at' may be used [-Werror=dangling-pointer=] 247 | m_timer->pop (m_tv); | ~~~~~~~~~~~~~^~~~~~ and a lot more dangling-pointer diagnostics during bootstrap. When trying to avoid control DCE for all clobber kinds we end up miscompiling g++.dg/pr64191.C. So the only working solution I found is handling indirect clobbers different from direct ones with regard to control DCE. Bootstrapped on x86_64-unknown-linux-gnu, testing in progress. Thoughts? Thanks, Richard. 2022-02-15 Richard Biener PR tree-optimization/96881 * tree-ssa-dce.cc (mark_stmt_if_obviously_necessary): Comment CLOBBER handling. (eliminate_unnecessary_stmts): Check that we preserved control parents before retaining a CLOBBER. (perform_tree_ssa_dce): Pass down aggressive flag to eliminate_unnecessary_stmts. * g++.dg/torture/pr96881-1.C: New testcase. * g++.dg/torture/pr96881-2.C: Likewise. --- gcc/testsuite/g++.dg/torture/pr96881-1.C | 37 ++++++++++++++++++++++++ gcc/testsuite/g++.dg/torture/pr96881-2.C | 37 ++++++++++++++++++++++++ gcc/tree-ssa-dce.cc | 13 ++++++--- 3 files changed, 83 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/g++.dg/torture/pr96881-1.C create mode 100644 gcc/testsuite/g++.dg/torture/pr96881-2.C diff --git a/gcc/testsuite/g++.dg/torture/pr96881-1.C b/gcc/testsuite/g++.dg/torture/pr96881-1.C new file mode 100644 index 00000000000..1c182e6a8b3 --- /dev/null +++ b/gcc/testsuite/g++.dg/torture/pr96881-1.C @@ -0,0 +1,37 @@ +/* { dg-do run } */ + +struct S { int s; ~S () {} } s; + +void __attribute__((noipa)) +foo (struct S *s, int flag) +{ + s->s = 1; + // We have to makes sure to not make the inlined CLOBBER + // unconditional but we have to remove it to be able + // to elide the branch + if (!flag) + return; + s->~S(); +} + +void __attribute__((noipa)) +bar (struct S *s, int flag) +{ + s->s = 1; + // CD-DCE chooses an arbitrary path, try to present it + // with all variants + if (flag) + s->~S(); +} + +int +main () +{ + foo (&s, 0); + if (s.s != 1) + __builtin_abort (); + bar (&s, 0); + if (s.s != 1) + __builtin_abort (); + return 0; +} diff --git a/gcc/testsuite/g++.dg/torture/pr96881-2.C b/gcc/testsuite/g++.dg/torture/pr96881-2.C new file mode 100644 index 00000000000..35c788791e8 --- /dev/null +++ b/gcc/testsuite/g++.dg/torture/pr96881-2.C @@ -0,0 +1,37 @@ +/* { dg-do run } */ + +struct S { int s; ~S () {} } s; + +void __attribute__((noipa)) +foo (int flag) +{ + s.s = 1; + // We have to makes sure to not make the inlined CLOBBER + // unconditional but we have to remove it to be able + // to elide the branch + if (!flag) + return; + s.~S(); +} + +void __attribute__((noipa)) +bar (int flag) +{ + s.s = 1; + // CD-DCE chooses an arbitrary path, try to present it + // with all variants + if (flag) + s.~S(); +} + +int +main () +{ + foo (0); + if (s.s != 1) + __builtin_abort (); + bar (0); + if (s.s != 1) + __builtin_abort (); + return 0; +} diff --git a/gcc/tree-ssa-dce.cc b/gcc/tree-ssa-dce.cc index f1034878eaf..9aefdc1c8d3 100644 --- a/gcc/tree-ssa-dce.cc +++ b/gcc/tree-ssa-dce.cc @@ -284,7 +284,10 @@ mark_stmt_if_obviously_necessary (gimple *stmt, bool aggressive) break; case GIMPLE_ASSIGN: - if (gimple_clobber_p (stmt)) + /* Mark indirect CLOBBERs to be lazily removed if their SSA operands + do not prevail. That also makes control flow leading to them + not necessary in aggressive mode. */ + if (gimple_clobber_p (stmt) && !zero_ssa_operands (stmt, SSA_OP_USE)) return; break; @@ -1272,7 +1275,7 @@ maybe_optimize_arith_overflow (gimple_stmt_iterator *gsi, contributes nothing to the program, and can be deleted. */ static bool -eliminate_unnecessary_stmts (void) +eliminate_unnecessary_stmts (bool aggressive) { bool something_changed = false; basic_block bb; @@ -1366,7 +1369,9 @@ eliminate_unnecessary_stmts (void) break; } } - if (!dead) + if (!dead + && (!aggressive + || bitmap_bit_p (visited_control_parents, bb->index))) { bitmap_clear (debug_seen); continue; @@ -1876,7 +1881,7 @@ perform_tree_ssa_dce (bool aggressive) propagate_necessity (aggressive); BITMAP_FREE (visited); - something_changed |= eliminate_unnecessary_stmts (); + something_changed |= eliminate_unnecessary_stmts (aggressive); something_changed |= cfg_altered; /* We do not update postdominators, so free them unconditionally. */