Re: [RFC][IPA-VRP] Re-factor tree-vrp to factor out common code

[kugan <kugan.vivekanandarajah@linaro.org>]

[-- Attachment #1: Type: text/plain, Size: 1651 bytes --]

Hi Richard,

Thanks for the review.

On 22/07/16 22:49, Richard Biener wrote:
> On Fri, Jul 22, 2016 at 2:27 PM, kugan
> <kugan.vivekanandarajah@linaro.org> wrote:
>> Hi,
>>
>> Now that early vrp is moved as part of tree-vrp, there is only minimal
>> interface tree-vrp should expose for ipa-vrp. However, I have not found the
>> right place to place struct value_range (with GTY marker) and enum
>> value_range_type.
>>
>> enum value_range_type is needed in tree-ssanames.[h|c] and in all the places
>> where get|set_range_info is used.
>>
>> struct value_range is needed in ipa-prop.h, therefore all the places
>> ipa-prop.h is included.
>>
>> One option is to place it in tree-vrp.h and expose this to GTY
>> infrastructure. Then include it in all the places we need any of these
>> types. It is in lots of c files.
>>
>> I have now placed both in tree.h. Even though that compiles, I am not
>> convinced that is the right place.
>
> I'm convinced it is not ;)

I totally agree. I just kept it their for the time being.

>
> As we had value_range_type in tree-ssanames.h why not put value_range there?
>
For IPA_VRP, we now need value_range used in ipa-prop.h (in ipa-vrp 
patch). Based on this, attached patch now adds struct value_range to 
tree-ssanames.h and fixes the header files. Please note that I also had 
to add other headers in few places due to the dependency. Are you OK 
with this ?

> Or simply put value_range into tree-vrp.h and leave value_range_type
> where it is.

In this case, we will have to add tree-vrp.h and tree-ssanames.h in 
those places.
>
> You no longer export vrp_finalize so no need to change it.

Thanks,
Kugan


[-- Attachment #2: 0004-Refactor-vrp.patch --]
[-- Type: text/x-patch, Size: 12625 bytes --]

From f75f2f836238e2e65615bec7ea2ce3a31257e204 Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Date: Tue, 21 Jun 2016 12:42:44 +1000
Subject: [PATCH 4/7] Refactor vrp

---
 gcc/cgraph.c               |  2 ++
 gcc/cgraphunit.c           |  1 +
 gcc/ipa-cp.c               |  3 +++
 gcc/ipa-devirt.c           |  2 ++
 gcc/ipa-inline-transform.c |  3 +++
 gcc/ipa-inline.c           |  2 ++
 gcc/ipa-profile.c          |  2 ++
 gcc/ipa-utils.c            |  2 ++
 gcc/ipa.c                  |  1 +
 gcc/lto/lto-partition.c    |  1 +
 gcc/lto/lto.c              |  2 ++
 gcc/toplev.c               |  2 ++
 gcc/trans-mem.c            |  1 +
 gcc/tree-ssanames.h        | 29 ++++++++++++++++++++++++++-
 gcc/tree-vrp.c             | 50 +++++++++++-----------------------------------
 gcc/tree-vrp.h             | 23 +++++++++++++++++++++
 16 files changed, 87 insertions(+), 39 deletions(-)
 create mode 100644 gcc/tree-vrp.h

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index e256dd0..01fbf5e 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -49,6 +49,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "value-prof.h"
 #include "ipa-utils.h"
 #include "symbol-summary.h"
+#include "stringpool.h"
+#include "tree-ssanames.h"
 #include "ipa-prop.h"
 #include "ipa-inline.h"
 #include "cfgloop.h"
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 4bfcad7..bf85356 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -190,6 +190,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "toplev.h"
 #include "debug.h"
 #include "symbol-summary.h"
+#include "tree-ssanames.h"
 #include "ipa-prop.h"
 #include "gimple-pretty-print.h"
 #include "plugin.h"
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 4b7f6bb..7e08a31 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -114,6 +114,9 @@ along with GCC; see the file COPYING3.  If not see
 #include "fold-const.h"
 #include "gimple-fold.h"
 #include "symbol-summary.h"
+#include "tree-ssa-alias.h"
+#include "stringpool.h"
+#include "tree-ssanames.h"
 #include "ipa-prop.h"
 #include "tree-pretty-print.h"
 #include "tree-inline.h"
diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
index 2cf018b..cdc539c 100644
--- a/gcc/ipa-devirt.c
+++ b/gcc/ipa-devirt.c
@@ -122,6 +122,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "ipa-utils.h"
 #include "gimple-fold.h"
 #include "symbol-summary.h"
+#include "stringpool.h"
+#include "tree-ssanames.h"
 #include "ipa-prop.h"
 #include "ipa-inline.h"
 #include "demangle.h"
diff --git a/gcc/ipa-inline-transform.c b/gcc/ipa-inline-transform.c
index f6b7d41..ea7c9c7 100644
--- a/gcc/ipa-inline-transform.c
+++ b/gcc/ipa-inline-transform.c
@@ -39,6 +39,9 @@ along with GCC; see the file COPYING3.  If not see
 #include "cgraph.h"
 #include "tree-cfg.h"
 #include "symbol-summary.h"
+#include "tree-ssa-alias.h"
+#include "stringpool.h"
+#include "tree-ssanames.h"
 #include "ipa-prop.h"
 #include "ipa-inline.h"
 #include "tree-inline.h"
diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
index 5c9366a..3eac298 100644
--- a/gcc/ipa-inline.c
+++ b/gcc/ipa-inline.c
@@ -108,6 +108,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "params.h"
 #include "profile.h"
 #include "symbol-summary.h"
+#include "stringpool.h"
+#include "tree-ssanames.h"
 #include "ipa-prop.h"
 #include "ipa-inline.h"
 #include "ipa-utils.h"
diff --git a/gcc/ipa-profile.c b/gcc/ipa-profile.c
index da17bcd..e43c3c9 100644
--- a/gcc/ipa-profile.c
+++ b/gcc/ipa-profile.c
@@ -62,6 +62,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "value-prof.h"
 #include "tree-inline.h"
 #include "symbol-summary.h"
+#include "stringpool.h"
+#include "tree-ssanames.h"
 #include "ipa-prop.h"
 #include "ipa-inline.h"
 
diff --git a/gcc/ipa-utils.c b/gcc/ipa-utils.c
index 5eb7d5f..df9f178 100644
--- a/gcc/ipa-utils.c
+++ b/gcc/ipa-utils.c
@@ -32,6 +32,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "splay-tree.h"
 #include "ipa-utils.h"
 #include "symbol-summary.h"
+#include "stringpool.h"
+#include "tree-ssanames.h"
 #include "ipa-prop.h"
 #include "ipa-inline.h"
 
diff --git a/gcc/ipa.c b/gcc/ipa.c
index 6722d3b..b8c55c0 100644
--- a/gcc/ipa.c
+++ b/gcc/ipa.c
@@ -32,6 +32,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-iterator.h"
 #include "ipa-utils.h"
 #include "symbol-summary.h"
+#include "tree-ssanames.h"
 #include "ipa-prop.h"
 #include "ipa-inline.h"
 #include "dbgcnt.h"
diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c
index 453343a..ca0fa24 100644
--- a/gcc/lto/lto-partition.c
+++ b/gcc/lto/lto-partition.c
@@ -31,6 +31,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "lto-streamer.h"
 #include "params.h"
 #include "symbol-summary.h"
+#include "tree-ssanames.h"
 #include "ipa-prop.h"
 #include "ipa-inline.h"
 #include "lto-partition.h"
diff --git a/gcc/lto/lto.c b/gcc/lto/lto.c
index af735cb..ea81931 100644
--- a/gcc/lto/lto.c
+++ b/gcc/lto/lto.c
@@ -36,6 +36,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "toplev.h"
 #include "stor-layout.h"
 #include "symbol-summary.h"
+#include "stringpool.h"
+#include "tree-ssanames.h"
 #include "ipa-prop.h"
 #include "common.h"
 #include "debug.h"
diff --git a/gcc/toplev.c b/gcc/toplev.c
index f51d2cb..4c973f0 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -71,6 +71,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "dwarf2out.h"
 #include "ipa-reference.h"
 #include "symbol-summary.h"
+#include "stringpool.h"
+#include "tree-ssanames.h"
 #include "ipa-prop.h"
 #include "gcse.h"
 #include "tree-chkp.h"
diff --git a/gcc/trans-mem.c b/gcc/trans-mem.c
index 2a6e101..3137f80 100644
--- a/gcc/trans-mem.c
+++ b/gcc/trans-mem.c
@@ -49,6 +49,7 @@
 #include "params.h"
 #include "langhooks.h"
 #include "cfgloop.h"
+#include "tree-ssanames.h"
 #include "tree-ssa-address.h"
 
 
diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
index c81b1a1..d4a8cc3 100644
--- a/gcc/tree-ssanames.h
+++ b/gcc/tree-ssanames.h
@@ -65,7 +65,34 @@ struct GTY ((variable_size)) range_info_def {
 
 /* Type of value ranges.  See value_range_d In tree-vrp.c for a
    description of these types.  */
-enum value_range_type { VR_UNDEFINED, VR_RANGE, VR_ANTI_RANGE, VR_VARYING };
+enum value_range_type { VR_UNDEFINED, VR_RANGE,
+			VR_ANTI_RANGE, VR_VARYING, VR_LAST };
+
+/* Range of values that can be associated with an SSA_NAME after VRP
+   has executed.  */
+struct GTY(()) value_range
+{
+  /* Lattice value represented by this range.  */
+  enum value_range_type type;
+
+  /* Minimum and maximum values represented by this range.  These
+     values should be interpreted as follows:
+
+	- If TYPE is VR_UNDEFINED or VR_VARYING then MIN and MAX must
+	  be NULL.
+
+	- If TYPE == VR_RANGE then MIN holds the minimum value and
+	  MAX holds the maximum value of the range [MIN, MAX].
+
+	- If TYPE == ANTI_RANGE the variable is known to NOT
+	  take any values in the range [MIN, MAX].  */
+  tree min;
+  tree max;
+
+  /* Set of SSA names whose value ranges are equivalent to this one.
+     This set is only valid when TYPE is VR_RANGE or VR_ANTI_RANGE.  */
+  bitmap equiv;
+};
 
 /* Sets the value range to SSA.  */
 extern void set_range_info (tree, enum value_range_type, const wide_int_ref &,
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index edaacf2..a318d39 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -59,32 +59,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "target.h"
 #include "case-cfn-macros.h"
 #include "alloc-pool.h"
-
-/* Range of values that can be associated with an SSA_NAME after VRP
-   has executed.  */
-struct value_range
-{
-  /* Lattice value represented by this range.  */
-  enum value_range_type type;
-
-  /* Minimum and maximum values represented by this range.  These
-     values should be interpreted as follows:
-
-	- If TYPE is VR_UNDEFINED or VR_VARYING then MIN and MAX must
-	  be NULL.
-
-	- If TYPE == VR_RANGE then MIN holds the minimum value and
-	  MAX holds the maximum value of the range [MIN, MAX].
-
-	- If TYPE == ANTI_RANGE the variable is known to NOT
-	  take any values in the range [MIN, MAX].  */
-  tree min;
-  tree max;
-
-  /* Set of SSA names whose value ranges are equivalent to this one.
-     This set is only valid when TYPE is VR_RANGE or VR_ANTI_RANGE.  */
-  bitmap equiv;
-};
+#include "tree-vrp.h"
 
 #define VR_INITIALIZER { VR_UNDEFINED, NULL_TREE, NULL_TREE, NULL }
 
@@ -108,8 +83,6 @@ live_on_edge (edge e, tree name)
 /* Local functions.  */
 static int compare_values (tree val1, tree val2);
 static int compare_values_warnv (tree val1, tree val2, bool *);
-static void vrp_meet (value_range *, value_range *);
-static void vrp_intersect_ranges (value_range *, value_range *);
 static tree vrp_evaluate_conditional_warnv_with_ops (enum tree_code,
 						     tree, tree, bool, bool *,
 						     bool *);
@@ -4605,7 +4578,7 @@ compare_range_with_value (enum tree_code comp, value_range *vr, tree val,
 
 /* Debugging dumps.  */
 
-void dump_value_range (FILE *, value_range *);
+void dump_value_range (FILE *, const value_range *);
 void debug_value_range (value_range *);
 void dump_all_value_ranges (FILE *);
 void debug_all_value_ranges (void);
@@ -4616,7 +4589,7 @@ void debug_vr_equiv (bitmap);
 /* Dump value range VR to FILE.  */
 
 void
-dump_value_range (FILE *file, value_range *vr)
+dump_value_range (FILE *file, const value_range *vr)
 {
   if (vr == NULL)
     fprintf (file, "[]");
@@ -8447,7 +8420,7 @@ intersect_ranges (enum value_range_type *vr0type,
 /* Intersect the two value-ranges *VR0 and *VR1 and store the result
    in *VR0.  This may not be the smallest possible such range.  */
 
-static void
+void
 vrp_intersect_ranges_1 (value_range *vr0, value_range *vr1)
 {
   value_range saved;
@@ -8499,7 +8472,7 @@ vrp_intersect_ranges_1 (value_range *vr0, value_range *vr1)
     bitmap_copy (vr0->equiv, vr1->equiv);
 }
 
-static void
+void
 vrp_intersect_ranges (value_range *vr0, value_range *vr1)
 {
   if (dump_file && (dump_flags & TDF_DETAILS))
@@ -8524,7 +8497,7 @@ vrp_intersect_ranges (value_range *vr0, value_range *vr1)
    may not be the smallest possible such range.  */
 
 static void
-vrp_meet_1 (value_range *vr0, value_range *vr1)
+vrp_meet_1 (value_range *vr0, const value_range *vr1)
 {
   value_range saved;
 
@@ -8596,8 +8569,8 @@ vrp_meet_1 (value_range *vr0, value_range *vr1)
     bitmap_clear (vr0->equiv);
 }
 
-static void
-vrp_meet (value_range *vr0, value_range *vr1)
+void
+vrp_meet (value_range *vr0, const value_range *vr1)
 {
   if (dump_file && (dump_flags & TDF_DETAILS))
     {
@@ -10171,7 +10144,7 @@ finalize_jump_threads (void)
 /* Traverse all the blocks folding conditionals with known ranges.  */
 
 static void
-vrp_finalize (bool warn_array_bounds_p)
+vrp_finalize (bool jump_thread_p, bool warn_array_bounds_p)
 {
   size_t i;
 
@@ -10212,7 +10185,8 @@ vrp_finalize (bool warn_array_bounds_p)
 
   /* We must identify jump threading opportunities before we release
      the datastructures built by VRP.  */
-  identify_jump_threads ();
+  if (jump_thread_p)
+    identify_jump_threads ();
 
   /* Free allocated memory.  */
   free (vr_value);
@@ -10296,7 +10270,7 @@ execute_vrp (bool warn_array_bounds_p)
 
   vrp_initialize ();
   ssa_propagate (vrp_visit_stmt, vrp_visit_phi_node);
-  vrp_finalize (warn_array_bounds_p);
+  vrp_finalize (true, warn_array_bounds_p);
 
   free_numbers_of_iterations_estimates (cfun);
 
diff --git a/gcc/tree-vrp.h b/gcc/tree-vrp.h
new file mode 100644
index 0000000..38aacf5
--- /dev/null
+++ b/gcc/tree-vrp.h
@@ -0,0 +1,23 @@
+/* Support routines for Value Range Propagation (VRP).
+   Copyright (C) Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+extern void vrp_intersect_ranges (value_range *vr0, value_range *vr1);
+extern void vrp_meet (value_range *vr0, const value_range *vr1);
+extern void dump_value_range (FILE *, const value_range *);
+
-- 
1.9.1