[RFC] Remove first_pass_instance from pass_vrp

[Tom de Vries <Tom_deVries at mentor dot com>]

Hi,

[ See also related discussion at 
https://gcc.gnu.org/ml/gcc-patches/2012-07/msg00452.html ]

this patch removes the usage of first_pass_instance from pass_vrp.

the patch:
- limits itself to pass_vrp, but my intention is to remove all
  usage of first_pass_instance
- lacks an update to gdbhooks.py

Modifying the pass behaviour depending on the instance number, as 
first_pass_instance does, break compositionality of the pass list. In 
other words, adding a pass instance in a pass list may change the 
behaviour of another instance of that pass in the pass list. Which 
obviously makes it harder to understand and change the pass list. [ I've 
filed this issue as PR68247 - Remove pass_first_instance ]

The solution is to make the difference in behaviour explicit in the pass 
list, and no longer change behaviour depending on instance number.

One obvious possible fix is to create a duplicate pass with a different 
name, say 'pass_vrp_warn_array_bounds':
...
  NEXT_PASS (pass_vrp_warn_array_bounds);
  ...
  NEXT_PASS (pass_vrp);
...

But, AFAIU that requires us to choose a different dump-file name for 
each pass. And choosing vrp1 and vrp2 as new dump-file names still means 
that -fdump-tree-vrp no longer works (which was mentioned as drawback 
here: https://gcc.gnu.org/ml/gcc-patches/2012-07/msg00453.html ).

This patch instead makes pass creation parameterizable. So in the pass 
list, we use:
...
  NEXT_PASS_WITH_ARG (pass_vrp, true /* warn_array_bounds_p */);
  ...
  NEXT_PASS_WITH_ARG (pass_vrp, false /* warn_array_bounds_p */);
...

This approach gives us clarity in the pass list, similar to using a 
duplicate pass 'pass_vrp_warn_array_bounds'.

But it also means -fdump-tree-vrp still works as before.

Good idea? Other comments?

Thanks,
- Tom

Remove first_pass_instance from pass_vrp

---
 gcc/gen-pass-instances.awk | 32 ++++++++++++++++++++++----------
 gcc/pass_manager.h         |  2 ++
 gcc/passes.c               | 20 ++++++++++++++++++++
 gcc/passes.def             |  4 ++--
 gcc/tree-pass.h            |  3 ++-
 gcc/tree-vrp.c             | 22 ++++++++++++----------
 6 files changed, 60 insertions(+), 23 deletions(-)

diff --git a/gcc/gen-pass-instances.awk b/gcc/gen-pass-instances.awk
index cbfaa86..c77bd64 100644
--- a/gcc/gen-pass-instances.awk
+++ b/gcc/gen-pass-instances.awk
@@ -43,7 +43,7 @@ function handle_line()
 	line = $0;
 
 	# Find call expression.
-	call_starts_at = match(line, /NEXT_PASS \(.+\)/);
+	call_starts_at = match(line, /NEXT_PASS(_WITH_ARG)? \(.+\)/);
 	if (call_starts_at == 0)
 	{
 		print line;
@@ -53,23 +53,28 @@ function handle_line()
 	# Length of the call expression.
 	len_of_call = RLENGTH;
 
-	len_of_start = length("NEXT_PASS (");
 	len_of_open = length("(");
 	len_of_close = length(")");
 
-	# Find pass_name argument
-	len_of_pass_name = len_of_call - (len_of_start + len_of_close);
-	pass_starts_at = call_starts_at + len_of_start;
-	pass_name = substr(line, pass_starts_at, len_of_pass_name);
-
 	# Find call expression prefix (until and including called function)
-	prefix_len = pass_starts_at - 1 - len_of_open;
-	prefix = substr(line, 1, prefix_len);
+	match(line, /NEXT_PASS(_WITH_ARG)? /)
+	len_of_call_name = RLENGTH
+	prefix_len = call_starts_at + len_of_call_name - 1
+	prefix = substr(line, 1, prefix_len)
 
 	# Find call expression postfix
 	postfix_starts_at = call_starts_at + len_of_call;
 	postfix = substr(line, postfix_starts_at);
 
+	args_starts_at = prefix_len + 1 + len_of_open;
+	len_of_args = postfix_starts_at - args_starts_at - len_of_close;
+	args_str = substr(line, args_starts_at, len_of_args);
+	split(args_str, args, ",");
+
+	# Find pass_name argument, an optional with_arg argument
+	pass_name = args[1];
+	with_arg = args[2];
+
 	# Set pass_counts
 	if (pass_name in pass_counts)
 		pass_counts[pass_name]++;
@@ -79,7 +84,14 @@ function handle_line()
 	pass_num = pass_counts[pass_name];
 
 	# Print call expression with extra pass_num argument
-	printf "%s(%s, %s)%s\n", prefix, pass_name, pass_num, postfix;
+	printf "%s(", prefix;
+	printf "%s", pass_name;
+	printf ", %s", pass_num;
+	if (with_arg)
+	{
+		printf ", %s", with_arg;
+	}
+	printf ")%s\n", postfix;
 }
 
 { handle_line() }
diff --git a/gcc/pass_manager.h b/gcc/pass_manager.h
index 7d539e4..a8199e2 100644
--- a/gcc/pass_manager.h
+++ b/gcc/pass_manager.h
@@ -120,6 +120,7 @@ private:
 #define PUSH_INSERT_PASSES_WITHIN(PASS)
 #define POP_INSERT_PASSES()
 #define NEXT_PASS(PASS, NUM) opt_pass *PASS ## _ ## NUM
+#define NEXT_PASS_WITH_ARG(PASS, NUM, ARG) NEXT_PASS (PASS, NUM)
 #define TERMINATE_PASS_LIST()
 
 #include "pass-instances.def"
@@ -128,6 +129,7 @@ private:
 #undef PUSH_INSERT_PASSES_WITHIN
 #undef POP_INSERT_PASSES
 #undef NEXT_PASS
+#undef NEXT_PASS_WITH_ARG
 #undef TERMINATE_PASS_LIST
 
 }; // class pass_manager
diff --git a/gcc/passes.c b/gcc/passes.c
index dd8d00a..0fd365e 100644
--- a/gcc/passes.c
+++ b/gcc/passes.c
@@ -81,6 +81,12 @@ opt_pass::clone ()
   internal_error ("pass %s does not support cloning", name);
 }
 
+opt_pass *
+opt_pass::clone_with_arg (bool)
+{
+  internal_error ("pass %s does not support cloning", name);
+}
+
 bool
 opt_pass::gate (function *)
 {
@@ -1572,6 +1578,19 @@ pass_manager::pass_manager (context *ctxt)
     p = next_pass_1 (p, PASS ## _ ## NUM, PASS ## _1);  \
   } while (0)
 
+#define NEXT_PASS_WITH_ARG(PASS, NUM, ARG)			\
+    do {							\
+      gcc_assert (NULL == PASS ## _ ## NUM);			\
+      if ((NUM) == 1)						\
+	PASS ## _1 = make_##PASS (m_ctxt, ARG);			\
+	else							\
+	  {							\
+	    gcc_assert (PASS ## _1);				\
+	    PASS ## _ ## NUM = PASS ## _1->clone_with_arg (ARG);	\
+	  }							\
+	  p = next_pass_1 (p, PASS ## _ ## NUM, PASS ## _1);	\
+    } while (0)
+
 #define TERMINATE_PASS_LIST() \
   *p = NULL;
 
@@ -1581,6 +1600,7 @@ pass_manager::pass_manager (context *ctxt)
 #undef PUSH_INSERT_PASSES_WITHIN
 #undef POP_INSERT_PASSES
 #undef NEXT_PASS
+#undef NEXT_PASS_WITH_ARG
 #undef TERMINATE_PASS_LIST
 
   /* Register the passes with the tree dump code.  */
diff --git a/gcc/passes.def b/gcc/passes.def
index c0ab6b9..2649d67 100644
--- a/gcc/passes.def
+++ b/gcc/passes.def
@@ -171,7 +171,7 @@ along with GCC; see the file COPYING3.  If not see
       NEXT_PASS (pass_return_slot);
       NEXT_PASS (pass_fre);
       NEXT_PASS (pass_merge_phi);
-      NEXT_PASS (pass_vrp);
+      NEXT_PASS_WITH_ARG (pass_vrp, true /* warn_array_bounds_p */);
       NEXT_PASS (pass_chkp_opt);
       NEXT_PASS (pass_dce);
       NEXT_PASS (pass_stdarg);
@@ -280,7 +280,7 @@ along with GCC; see the file COPYING3.  If not see
       NEXT_PASS (pass_tracer);
       NEXT_PASS (pass_dominator);
       NEXT_PASS (pass_strlen);
-      NEXT_PASS (pass_vrp);
+      NEXT_PASS_WITH_ARG (pass_vrp, false /* warn_array_bounds_p */);
       /* The only const/copy propagation opportunities left after
 	 DOM and VRP should be due to degenerate PHI nodes.  So rather than
 	 run the full propagators, run a specialized pass which
diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h
index 49e22a9..0e330dd 100644
--- a/gcc/tree-pass.h
+++ b/gcc/tree-pass.h
@@ -83,6 +83,7 @@ public:
 
      The default implementation prints an error message and aborts.  */
   virtual opt_pass *clone ();
+  virtual opt_pass *clone_with_arg (bool);
 
   /* This pass and all sub-passes are executed only if the function returns
      true.  The default implementation returns true.  */
@@ -439,7 +440,7 @@ extern gimple_opt_pass *make_pass_fre (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_check_data_deps (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_copy_prop (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_isolate_erroneous_paths (gcc::context *ctxt);
-extern gimple_opt_pass *make_pass_vrp (gcc::context *ctxt);
+extern gimple_opt_pass *make_pass_vrp (gcc::context *ctxt, bool);
 extern gimple_opt_pass *make_pass_uncprop (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_return_slot (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_reassoc (gcc::context *ctxt);
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index e2393e4..0ff60fd 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -10183,7 +10183,7 @@ finalize_jump_threads (void)
 /* Traverse all the blocks folding conditionals with known ranges.  */
 
 static void
-vrp_finalize (void)
+vrp_finalize (bool warn_array_bounds_p)
 {
   size_t i;
 
@@ -10199,7 +10199,7 @@ vrp_finalize (void)
   substitute_and_fold (op_with_constant_singleton_value_range,
 		       vrp_fold_stmt, false);
 
-  if (warn_array_bounds && first_pass_instance)
+  if (warn_array_bounds && warn_array_bounds_p)
     check_all_array_refs ();
 
   /* We must identify jump threading opportunities before we release
@@ -10289,7 +10289,7 @@ vrp_finalize (void)
    probabilities to aid branch prediction.  */
 
 static unsigned int
-execute_vrp (void)
+execute_vrp (bool warn_array_bounds_p)
 {
   int i;
   edge e;
@@ -10313,7 +10313,7 @@ execute_vrp (void)
 
   vrp_initialize ();
   ssa_propagate (vrp_visit_stmt, vrp_visit_phi_node);
-  vrp_finalize ();
+  vrp_finalize (warn_array_bounds_p);
 
   free_numbers_of_iterations_estimates (cfun);
 
@@ -10385,21 +10385,23 @@ const pass_data pass_data_vrp =
 class pass_vrp : public gimple_opt_pass
 {
 public:
-  pass_vrp (gcc::context *ctxt)
-    : gimple_opt_pass (pass_data_vrp, ctxt)
+  pass_vrp (gcc::context *ctxt, bool warn_array_bounds_p)
+    : gimple_opt_pass (pass_data_vrp, ctxt), warn_array_bounds_p(warn_array_bounds_p)
   {}
 
   /* opt_pass methods: */
-  opt_pass * clone () { return new pass_vrp (m_ctxt); }
+  opt_pass * clone (bool warn_array_bounds_p) { return new pass_vrp (m_ctxt, warn_array_bounds_p); }
   virtual bool gate (function *) { return flag_tree_vrp != 0; }
-  virtual unsigned int execute (function *) { return execute_vrp (); }
+  virtual unsigned int execute (function *) { return execute_vrp (warn_array_bounds_p); }
 
+ private:
+  bool warn_array_bounds_p;
 }; // class pass_vrp
 
 } // anon namespace
 
 gimple_opt_pass *
-make_pass_vrp (gcc::context *ctxt)
+make_pass_vrp (gcc::context *ctxt, bool warn_array_bounds_p)
 {
-  return new pass_vrp (ctxt);
+  return new pass_vrp (ctxt, warn_array_bounds_p);
 }