From patchwork Fri Feb 4 09:39:27 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 50791 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 785093858D37 for ; Fri, 4 Feb 2022 09:39:58 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 785093858D37 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1643967598; bh=WZwstiqI8p+EcVP0Xp3jCLxnKCdYR4s6Wce3edy10yI=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=aGHQZHBGt7jV57gqz6S4Pa/q/PLJX1Gi1ZZz86eVSDCCYwxjarqK6m2oheXAmLnuS W7Na2MEE8ReH29kF9myG3twfTIlh1AlRlcWJSaXIz38KVOeypSeBXmIkff2aQ1dvsw e6QgTF7+04UdU87Wlvc0cyYZiwFBPCWUlz8w7aIw= 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 156203858D28 for ; Fri, 4 Feb 2022 09:39:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 156203858D28 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 F3FA91F382; Fri, 4 Feb 2022 09:39: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 DBFD5139D2; Fri, 4 Feb 2022 09:39:27 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id CM16NE/0/GGZAQAAMHmgww (envelope-from ); Fri, 04 Feb 2022 09:39:27 +0000 Date: Fri, 4 Feb 2022 10:39:27 +0100 (CET) To: gcc-patches@gcc.gnu.org Subject: [PATCH][RFC] tree-optimization/104373 - early diagnostic on unreachable code MIME-Version: 1.0 Message-Id: <20220204093927.DBFD5139D2@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" The following improves early uninit diagnostics by computing edge reachability using VN and ignoring unreachable blocks when looking for uninitialized uses. To not ICE with -fdump-tree-all the early uninit pass needs a dumpfile since VN tries to dump statistics. Bootstrapped on x86_64-unknown-linux-gnu, testing in progress. In the PR I note that the early warn_printf and warn_nonnull_compare and warn_access might also benefit from reachability analysis which would probably mean a separate "reachability analysis" pass computing EDGE_EXECUTABLE conditionaly on any of the consuming diagnostics. I've went with this simpler proof-of-concept to gather feedback on the idea though. As of costs the patch cuts the most improtant part of VN, the alias VUSE->VDEF walking away, and relies on the non-iterative VN mode being O (n * something like log n) by design (but with quite a high constant factor). For the case of uninit I avoid the extra VN walk when optimizing since we only warn for always executed code there (but those cases could be expanded with VN and an adjusted CFG walk). The other early diagnostics likely warn everywhere even when optimizing so when expanding the idea we'd do the extra VN run even when optimizing and then the question arises, if we do it as a separate phase, whether we want to enable IL tranforms (not sure how much the early code relies on seeing none of those). Thus - feedback is welcome. The PR itself is a 12 regression so technically P1 but this is more a general improvement. In the audit log I also mention that for the specific testcase there's a cheaper way by doing a simple-mindend const/copy lattice and RPO walk in the uninit pass itself. Thanks, Richard. 2022-02-04 Richard Biener PR tree-optimization/104373 * tree-ssa-sccvn.h (do_rpo_vn): New export exposing the walk kind. * tree-ssa-sccvn.cc (do_rpo_vn): Export, get the default walk kind as argument. (run_rpo_vn): Adjust. (pass_fre::execute): Likewise. * tree-ssa-uninit.cc (warn_uninitialized_vars): Skip blocks not reachable. (execute_late_warn_uninitialized): Mark all edges as executable. (execute_early_warn_uninitialized): Use VN to compute executable edges. (pass_data_early_warn_uninitialized): Enable a dump file. --- gcc/testsuite/g++.dg/warn/Wuninitialized-32.C | 14 ++++++++ gcc/tree-ssa-sccvn.cc | 18 +++++----- gcc/tree-ssa-sccvn.h | 1 + gcc/tree-ssa-uninit.cc | 36 ++++++++++++++++--- 4 files changed, 54 insertions(+), 15 deletions(-) create mode 100644 gcc/testsuite/g++.dg/warn/Wuninitialized-32.C diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-32.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-32.C new file mode 100644 index 00000000000..8b02b5c6adb --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-32.C @@ -0,0 +1,14 @@ +// { dg-do compile } +// { dg-additional-options "-Wall" } + +void* operator new[](unsigned long, void* __p); + +struct allocator +{ + ~allocator(); +}; + +void *foo (void *p) +{ + return p ? new(p) allocator[1] : new allocator[1]; // { dg-bogus "uninitialized" } +} diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc index a03f0aae924..eb17549c185 100644 --- a/gcc/tree-ssa-sccvn.cc +++ b/gcc/tree-ssa-sccvn.cc @@ -7034,15 +7034,14 @@ eliminate_with_rpo_vn (bitmap inserted_exprs) return walker.eliminate_cleanup (); } -static unsigned +unsigned do_rpo_vn (function *fn, edge entry, bitmap exit_bbs, - bool iterate, bool eliminate); + bool iterate, bool eliminate, vn_lookup_kind kind); void run_rpo_vn (vn_lookup_kind kind) { - default_vn_walk_kind = kind; - do_rpo_vn (cfun, NULL, NULL, true, false); + do_rpo_vn (cfun, NULL, NULL, true, false, kind); /* ??? Prune requirement of these. */ constant_to_value_id = new hash_table (23); @@ -7740,11 +7739,12 @@ do_unwind (unwind_state *to, rpo_elim &avail) executed and iterate. If ELIMINATE is true then perform elimination, otherwise leave that to the caller. */ -static unsigned +unsigned do_rpo_vn (function *fn, edge entry, bitmap exit_bbs, - bool iterate, bool eliminate) + bool iterate, bool eliminate, vn_lookup_kind kind) { unsigned todo = 0; + default_vn_walk_kind = kind; /* We currently do not support region-based iteration when elimination is requested. */ @@ -8164,8 +8164,7 @@ do_rpo_vn (function *fn, edge entry, bitmap exit_bbs, unsigned do_rpo_vn (function *fn, edge entry, bitmap exit_bbs) { - default_vn_walk_kind = VN_WALKREWRITE; - unsigned todo = do_rpo_vn (fn, entry, exit_bbs, false, true); + unsigned todo = do_rpo_vn (fn, entry, exit_bbs, false, true, VN_WALKREWRITE); free_rpo_vn (); return todo; } @@ -8221,8 +8220,7 @@ pass_fre::execute (function *fun) if (iterate_p) loop_optimizer_init (AVOID_CFG_MODIFICATIONS); - default_vn_walk_kind = VN_WALKREWRITE; - todo = do_rpo_vn (fun, NULL, NULL, iterate_p, true); + todo = do_rpo_vn (fun, NULL, NULL, iterate_p, true, VN_WALKREWRITE); free_rpo_vn (); if (iterate_p) diff --git a/gcc/tree-ssa-sccvn.h b/gcc/tree-ssa-sccvn.h index f161b80417b..aeed07039b7 100644 --- a/gcc/tree-ssa-sccvn.h +++ b/gcc/tree-ssa-sccvn.h @@ -291,6 +291,7 @@ value_id_constant_p (unsigned int v) tree fully_constant_vn_reference_p (vn_reference_t); tree vn_nary_simplify (vn_nary_op_t); +unsigned do_rpo_vn (function *, edge, bitmap, bool, bool, vn_lookup_kind); unsigned do_rpo_vn (function *, edge, bitmap); void run_rpo_vn (vn_lookup_kind); unsigned eliminate_with_rpo_vn (bitmap); diff --git a/gcc/tree-ssa-uninit.cc b/gcc/tree-ssa-uninit.cc index 02e88d58e1f..6683ce62e07 100644 --- a/gcc/tree-ssa-uninit.cc +++ b/gcc/tree-ssa-uninit.cc @@ -38,8 +38,9 @@ along with GCC; see the file COPYING3. If not see #include "builtins.h" #include "calls.h" #include "gimple-range.h" - #include "gimple-predicate-analysis.h" +#include "domwalk.h" +#include "tree-ssa-sccvn.h" /* This implements the pass that does predicate aware warning on uses of possibly uninitialized variables. The pass first collects the set of @@ -986,7 +987,19 @@ warn_uninitialized_vars (bool wmaybe_uninit) basic_block bb; FOR_EACH_BB_FN (bb, cfun) { + edge_iterator ei; + edge e; + FOR_EACH_EDGE (e, ei, bb->preds) + if (e->flags & EDGE_EXECUTABLE) + break; + /* Skip unreachable blocks. For early analysis we use VN to + determine edge executability. */ + if (!e) + continue; + basic_block succ = single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun)); + /* ??? This could be improved when we use a greedy walk and have + some edges marked as not executable. */ wlims.always_executed = dominated_by_p (CDI_POST_DOMINATORS, succ, bb); if (wlims.always_executed) @@ -1319,6 +1332,11 @@ execute_late_warn_uninitialized (function *fun) calculate_dominance_info (CDI_DOMINATORS); calculate_dominance_info (CDI_POST_DOMINATORS); + + /* Mark all edges executable, warn_uninitialized_vars will skip + unreachable blocks. */ + set_all_edges_as_executable (fun); + /* Re-do the plain uninitialized variable check, as optimization may have straightened control flow. Do this first so that we don't accidentally get a "may be" warning when we'd have seen an "is" warning later. */ @@ -1388,7 +1406,7 @@ make_pass_late_warn_uninitialized (gcc::context *ctxt) } static unsigned int -execute_early_warn_uninitialized (void) +execute_early_warn_uninitialized (struct function *fun) { /* Currently, this pass runs always but execute_late_warn_uninitialized only runs with optimization. With @@ -1398,6 +1416,14 @@ execute_early_warn_uninitialized (void) calculate_dominance_info (CDI_DOMINATORS); calculate_dominance_info (CDI_POST_DOMINATORS); + /* Use VN in its cheapest incarnation and without doing any + elimination to compute edge reachability. Don't bother when + we only warn for unconditionally executed code though. */ + if (!optimize) + do_rpo_vn (fun, NULL, NULL, false, false, VN_NOWALK); + else + set_all_edges_as_executable (fun); + warn_uninitialized_vars (/*warn_maybe_uninitialized=*/!optimize); /* Post-dominator information cannot be reliably updated. Free it @@ -1412,7 +1438,7 @@ namespace { const pass_data pass_data_early_warn_uninitialized = { GIMPLE_PASS, /* type */ - "*early_warn_uninitialized", /* name */ + "early_warn_uninitialized", /* name */ OPTGROUP_NONE, /* optinfo_flags */ TV_TREE_UNINIT, /* tv_id */ PROP_ssa, /* properties_required */ @@ -1431,9 +1457,9 @@ public: /* opt_pass methods: */ virtual bool gate (function *) { return gate_warn_uninitialized (); } - virtual unsigned int execute (function *) + virtual unsigned int execute (function *fun) { - return execute_early_warn_uninitialized (); + return execute_early_warn_uninitialized (fun); } }; // class pass_early_warn_uninitialized