diff mbox series

[RFC] tree-optimization/104373 - early diagnostic on unreachable code

Message ID 20220204093927.DBFD5139D2@imap2.suse-dmz.suse.de
State New
Headers
Series [RFC] tree-optimization/104373 - early diagnostic on unreachable code |

Commit Message

Richard Biener Feb. 4, 2022, 9:39 a.m. UTC 
  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  <rguenther@suse.de>

	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 mbox series

Patch

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<vn_constant_hasher> (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