[PATCH] Simplify and split irange::copy_legacy_range into two functions.

Aldy Hernandez aldyh@redhat.com
Tue Oct 20 13:59:11 GMT 2020


copy_legacy_range was a small but complex function.  It was tricky
to understand, and easy to introduce bugs into it.  It also did
unnecessary work on some code paths.

This patch splits the function into two functions that are more
efficient and easier to read (copy_to_legacy and
copy_legacy_to_multi_range).

Pushed.

gcc/ChangeLog:

	* value-range.cc (irange::operator=): Split up call to
	copy_legacy_range into...
	(irange::copy_to_legacy): ...this.
	(irange::copy_legacy_to_multi_range): ...and this.
	(irange::copy_legacy_range): Remove.
	* value-range.h: Remove copoy_legacy_range.
	Add copy_legacy_to_multi_range and copy_to_legacy.
---
 gcc/value-range.cc | 72 +++++++++++++++++++++++++++-------------------
 gcc/value-range.h  |  3 +-
 2 files changed, 44 insertions(+), 31 deletions(-)

diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index cdcc6c65594..7847104050c 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -35,18 +35,14 @@ along with GCC; see the file COPYING3.  If not see
 irange &
 irange::operator= (const irange &src)
 {
-  if (legacy_mode_p () != src.legacy_mode_p ())
+  if (legacy_mode_p ())
     {
-      copy_legacy_range (src);
+      copy_to_legacy (src);
       return *this;
     }
-  if (legacy_mode_p ())
+  if (src.legacy_mode_p ())
     {
-      gcc_checking_assert (src.legacy_mode_p ());
-      m_num_ranges = src.m_num_ranges;
-      m_base[0] = src.m_base[0];
-      m_base[1] = src.m_base[1];
-      m_kind = src.m_kind;
+      copy_legacy_to_multi_range (src);
       return *this;
     }
 
@@ -81,44 +77,60 @@ irange::maybe_anti_range () const
 	  && upper_bound () == wi::max_value (precision, sign));
 }
 
-// Copy between a legacy and a multi-range, or vice-versa.
-
 void
-irange::copy_legacy_range (const irange &src)
+irange::copy_legacy_to_multi_range (const irange &src)
 {
-  gcc_checking_assert (src.legacy_mode_p () != legacy_mode_p ());
+  gcc_checking_assert (src.legacy_mode_p ());
+  gcc_checking_assert (!legacy_mode_p ());
   if (src.undefined_p ())
     set_undefined ();
   else if (src.varying_p ())
     set_varying (src.type ());
-  else if (src.kind () == VR_ANTI_RANGE)
-    {
-      if (src.legacy_mode_p () && !range_has_numeric_bounds_p (&src))
-	set_varying (src.type ());
-      else
-	set (src.min (), src.max (), VR_ANTI_RANGE);
-    }
-  else if (legacy_mode_p () && src.maybe_anti_range ())
-    {
-      int_range<3> tmp (src);
-      tmp.invert ();
-      set (tmp.min (), wide_int_to_tree (src.type (), tmp.upper_bound (0)),
-	   VR_ANTI_RANGE);
-    }
   else
     {
-      // If copying legacy to int_range, normalize any symbolics.
-      if (src.legacy_mode_p () && !range_has_numeric_bounds_p (&src))
+      if (range_has_numeric_bounds_p (&src))
+	set (src.min (), src.max (), src.kind ());
+      else
 	{
 	  value_range cst (src);
 	  cst.normalize_symbolics ();
+	  gcc_checking_assert (cst.varying_p () || cst.kind () == VR_RANGE);
 	  set (cst.min (), cst.max ());
-	  return;
 	}
-      set (src.min (), src.max ());
     }
 }
 
+// Copy any type of irange into a legacy.
+
+void
+irange::copy_to_legacy (const irange &src)
+{
+  gcc_checking_assert (legacy_mode_p ());
+  // Copy legacy to legacy.
+  if (src.legacy_mode_p ())
+    {
+      m_num_ranges = src.m_num_ranges;
+      m_base[0] = src.m_base[0];
+      m_base[1] = src.m_base[1];
+      m_kind = src.m_kind;
+      return;
+    }
+  // Copy multi-range to legacy.
+  if (src.undefined_p ())
+    set_undefined ();
+  else if (src.varying_p ())
+    set_varying (src.type ());
+  else if (src.maybe_anti_range ())
+    {
+      int_range<3> r (src);
+      r.invert ();
+      // Use tree variants to save on tree -> wi -> tree conversions.
+      set (r.tree_lower_bound (0), r.tree_upper_bound (0), VR_ANTI_RANGE);
+    }
+  else
+    set (src.tree_lower_bound (), src.tree_upper_bound ());
+}
+
 // Swap min/max if they are out of order.  Return TRUE if further
 // processing of the range is necessary, FALSE otherwise.
 
diff --git a/gcc/value-range.h b/gcc/value-range.h
index 63c96204cda..760ee772316 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -124,7 +124,8 @@ protected:
   wide_int legacy_upper_bound (unsigned) const;
   int value_inside_range (tree) const;
   bool maybe_anti_range () const;
-  void copy_legacy_range (const irange &);
+  void copy_to_legacy (const irange &);
+  void copy_legacy_to_multi_range (const irange &);
 
 private:
   unsigned char m_num_ranges;
-- 
2.26.2



More information about the Gcc-patches mailing list