diff mbox series

Tweaks to ranger API routines.

Message ID 185913b2-466b-b477-347c-c17e6c4c36b0@redhat.com
State New
Headers show
Series Tweaks to ranger API routines. | expand

Commit Message

Andrew MacLeod Oct. 28, 2020, 12:20 a.m. UTC
There are a couple of little things I've been meaning to get to.

1)  The API routines return TRUE when a range has been computed, and 
false when one cannot be. The range itself was uninitialized when false 
was returned, and this was a bit awkward by times.

Now, when false is returned, the range is also set to UNDEFINED... which 
also makes sense, and allows for some simpler flow in places.

2) A number of routines had carried forward a habit of asking for a 
range, and confirming TRUE was returned via
gcc_assert (range_of_expr (...));

It unclear to me if one can be guaranteed that the call WILL be made, 
and besides that, I don't particularly like the optics/model. So I 
audited the code and in hand with the above change, removed the spurious 
gcc_asserts, or changed them to an appropriate gcc_checking_assert on a 
value when appropriate.

bootstrapped on x86_64-pc-linux-gnu, both normal and 
--enable-checking=release,   no regressions, and pushed.

Andrew
diff mbox series

Patch

commit c25d317cf7d4ea8df0402feb939ce286e5f42988
Author: Andrew MacLeod <amacleod@redhat.com>
Date:   Tue Oct 27 20:13:46 2020 -0400

    Tweaks to ranger API routines.
    
    Remove the gcc_assert wrappers that contain statements that need to be
    executed.
    Audit routines to ensure range is set to UNDEFINED when false is returned.
    
            * gimple-range-gori.cc (gori_compute_cache::cache_stmt): Accumulate
            return values and only set cache when everything returned true.
            * gimple-range.cc (get_tree_range): Set the return range to UNDEFINED
            when the range isn't supported.
            (gimple_ranger::calc_stmt): Return varying if the type is supported,
            even if the stmt processing failed.  False otherwise.
            (range_of_builtin_ubsan_call): Don't use gcc_assert.
            (range_of_builtin_call): Ditto.
            (gimple_ranger::range_of_cond_expr): Ditto.
            (gimple_ranger::range_of_expr): Ditto
            (gimple_ranger::range_on_entry): Ditto.
            (gimple_ranger::range_on_exit): Ditto.
            (gimple_ranger::range_on_edge): DItto.
            (gimple_ranger::range_of_stmt): Don't use gcc_assert, and initialize
            return value to UNDEFINED.

diff --git a/gcc/gimple-range-gori.cc b/gcc/gimple-range-gori.cc
index de0f653860d..54385baa629 100644
--- a/gcc/gimple-range-gori.cc
+++ b/gcc/gimple-range-gori.cc
@@ -1313,13 +1313,15 @@  gori_compute_cache::cache_stmt (gimple *stmt)
   else if (tree cached_name = m_cache->same_cached_name (op1, op2))
     {
       tf_range op1_range, op2_range;
-      gcc_assert (m_cache->get_range (op1_range, op1, cached_name));
-      gcc_assert (m_cache->get_range (op2_range, op2, cached_name));
-      gcc_assert (logical_combine (r_true_side, code, m_bool_one,
-				   op1_range, op2_range));
-      gcc_assert (logical_combine (r_false_side, code, m_bool_zero,
-				   op1_range, op2_range));
-      m_cache->set_range (lhs, cached_name,
-			  tf_range (r_true_side, r_false_side));
+      bool ok = m_cache->get_range (op1_range, op1, cached_name);
+      ok = ok && m_cache->get_range (op2_range, op2, cached_name);
+      ok = ok && logical_combine (r_true_side, code, m_bool_one,
+				  op1_range, op2_range);
+      ok = ok && logical_combine (r_false_side, code, m_bool_zero,
+				  op1_range, op2_range);
+      gcc_checking_assert (ok);
+      if (ok)
+	m_cache->set_range (lhs, cached_name,
+			    tf_range (r_true_side, r_false_side));
     }
 }
diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
index f5c6a1ca620..cf979845acf 100644
--- a/gcc/gimple-range.cc
+++ b/gcc/gimple-range.cc
@@ -144,7 +144,7 @@  gimple_range_adjustment (irange &res, const gimple *stmt)
 }
 
 // Return a range in R for the tree EXPR.  Return true if a range is
-// representable.
+// representable, and UNDEFINED/false if not.
 
 bool
 get_tree_range (irange &r, tree expr)
@@ -157,7 +157,10 @@  get_tree_range (irange &r, tree expr)
 
   // Return false if the type isn't suported.
   if (!irange::supports_type_p (type))
-    return false;
+    {
+      r.set_undefined ();
+      return false;
+    }
 
   switch (TREE_CODE (expr))
     {
@@ -373,7 +376,8 @@  gimple_ranger::calc_stmt (irange &r, gimple *s, tree name)
     res = range_of_call (r, as_a<gcall *> (s));
   else if (is_a<gassign *> (s) && gimple_assign_rhs_code (s) == COND_EXPR)
     res = range_of_cond_expr (r, as_a<gassign *> (s));
-  else
+
+  if (!res)
     {
       // If no name is specified, try the expression kind.
       if (!name)
@@ -384,25 +388,24 @@  gimple_ranger::calc_stmt (irange &r, gimple *s, tree name)
 	  r.set_varying (t);
 	  return true;
 	}
+      if (!gimple_range_ssa_p (name))
+	return false;
       // We don't understand the stmt, so return the global range.
       r = gimple_range_global (name);
       return true;
     }
-  if (res)
+
+  if (r.undefined_p ())
+    return true;
+
+  // We sometimes get compatible types copied from operands, make sure
+  // the correct type is being returned.
+  if (name && TREE_TYPE (name) != r.type ())
     {
-      if (r.undefined_p ())
-	return true;
-      // We sometimes get compatible types copied from operands, make sure
-      // the correct type is being returned.
-      if (name && TREE_TYPE (name) != r.type ())
-	{
-	  gcc_checking_assert (range_compatible_p (r.type (),
-						   TREE_TYPE (name)));
-	  range_cast (r, TREE_TYPE (name));
-	}
-      return true;
+      gcc_checking_assert (range_compatible_p (r.type (), TREE_TYPE (name)));
+      range_cast (r, TREE_TYPE (name));
     }
-  return false;
+  return true;
 }
 
 // Calculate a range for range_op statement S and return it in R.  If any
@@ -582,8 +585,8 @@  range_of_builtin_ubsan_call (range_query &query, irange &r, gcall *call,
   int_range_max ir0, ir1;
   tree arg0 = gimple_call_arg (call, 0);
   tree arg1 = gimple_call_arg (call, 1);
-  gcc_assert (query.range_of_expr (ir0, arg0, call));
-  gcc_assert (query.range_of_expr (ir1, arg1, call));
+  query.range_of_expr (ir0, arg0, call);
+  query.range_of_expr (ir1, arg1, call);
 
   bool saved_flag_wrapv = flag_wrapv;
   // Pretend the arithmetic is wrapping.  If there is any overflow,
@@ -638,7 +641,7 @@  range_of_builtin_call (range_query &query, irange &r, gcall *call)
       prec = TYPE_PRECISION (TREE_TYPE (arg));
       mini = 0;
       maxi = prec;
-      gcc_assert (query.range_of_expr (r, arg, call));
+      query.range_of_expr (r, arg, call);
       // If arg is non-zero, then ffs or popcount are non-zero.
       if (!range_includes_zero_p (&r))
 	mini = 1;
@@ -682,7 +685,7 @@  range_of_builtin_call (range_query &query, irange &r, gcall *call)
 	    }
 	}
 
-      gcc_assert (query.range_of_expr (r, arg, call));
+      query.range_of_expr (r, arg, call);
       // From clz of minimum we can compute result maximum.
       if (r.constant_p ())
 	{
@@ -747,7 +750,7 @@  range_of_builtin_call (range_query &query, irange &r, gcall *call)
 		mini = -2;
 	    }
 	}
-      gcc_assert (query.range_of_expr (r, arg, call));
+      query.range_of_expr (r, arg, call);
       if (!r.undefined_p ())
 	{
 	  if (r.lower_bound () != 0)
@@ -864,9 +867,9 @@  gimple_ranger::range_of_cond_expr  (irange &r, gassign *s)
   if (!irange::supports_type_p (TREE_TYPE (op1)))
     return false;
 
-  gcc_assert (range_of_expr (cond_range, cond, s));
-  gcc_assert (range_of_expr (range1, op1, s));
-  gcc_assert (range_of_expr (range2, op2, s));
+  range_of_expr (cond_range, cond, s);
+  range_of_expr (range1, op1, s);
+  range_of_expr (range2, op2, s);
 
   // If the condition is known, choose the appropriate expression.
   if (cond_range.singleton_p ())
@@ -904,7 +907,7 @@  gimple_ranger::range_of_expr (irange &r, tree expr, gimple *stmt)
 
   // If name is defined in this block, try to get an range from S.
   if (def_stmt && gimple_bb (def_stmt) == bb)
-    gcc_assert (range_of_stmt (r, def_stmt, expr));
+    range_of_stmt (r, def_stmt, expr);
   else
     // Otherwise OP comes from outside this block, use range on entry.
     range_on_entry (r, bb, expr);
@@ -933,7 +936,7 @@  gimple_ranger::range_on_entry (irange &r, basic_block bb, tree name)
   gcc_checking_assert (gimple_range_ssa_p (name));
 
   // Start with any known range
-  gcc_assert (range_of_stmt (r, SSA_NAME_DEF_STMT (name), name));
+  range_of_stmt (r, SSA_NAME_DEF_STMT (name), name);
 
   // Now see if there is any on_entry value which may refine it.
   if (m_cache.block_range (entry_range, bb, name))
@@ -948,6 +951,7 @@  gimple_ranger::range_on_exit (irange &r, basic_block bb, tree name)
 {
   // on-exit from the exit block?
   gcc_checking_assert (bb != EXIT_BLOCK_PTR_FOR_FN (cfun));
+  gcc_checking_assert (gimple_range_ssa_p (name));
 
   gimple *s = last_stmt (bb);
   // If there is no statement in the block and this isn't the entry
@@ -956,7 +960,7 @@  gimple_ranger::range_on_exit (irange &r, basic_block bb, tree name)
   if (!s && bb != ENTRY_BLOCK_PTR_FOR_FN (cfun))
     range_on_entry (r, bb, name);
   else
-    gcc_assert (range_of_expr (r, name, s));
+    range_of_expr (r, name, s);
   gcc_checking_assert (r.undefined_p ()
 		       || range_compatible_p (r.type (), TREE_TYPE (name)));
 }
@@ -971,10 +975,7 @@  gimple_ranger::range_on_edge (irange &r, edge e, tree name)
 
   // PHI arguments can be constants, catch these here.
   if (!gimple_range_ssa_p (name))
-    {
-      gcc_assert (range_of_expr (r, name));
-      return true;
-    }
+    return range_of_expr (r, name);
 
   range_on_exit (r, e->src, name);
   gcc_checking_assert  (r.undefined_p ()
@@ -991,29 +992,32 @@  gimple_ranger::range_on_edge (irange &r, edge e, tree name)
 // provided it represents the SSA_NAME on the LHS of the statement.
 // It is only required if there is more than one lhs/output.  Check
 // the global cache for NAME first to see if the evaluation can be
-// avoided.  If a range cannot be calculated, return false.
+// avoided.  If a range cannot be calculated, return false and UNDEFINED.
 
 bool
 gimple_ranger::range_of_stmt (irange &r, gimple *s, tree name)
 {
-  // If no name, simply call the base routine.
+  r.set_undefined ();
+
   if (!name)
     name = gimple_get_lhs (s);
 
+  // If no name, simply call the base routine.
   if (!name)
     return calc_stmt (r, s, NULL_TREE);
 
-  gcc_checking_assert (TREE_CODE (name) == SSA_NAME &&
-		       irange::supports_type_p (TREE_TYPE (name)));
+  if (!gimple_range_ssa_p (name))
+    return false;
 
   // If this STMT has already been processed, return that value.
   if (m_cache.m_globals.get_global_range (r, name))
     return true;
+
   // Avoid infinite recursion by initializing global cache
   int_range_max tmp = gimple_range_global (name);
   m_cache.m_globals.set_global_range (name, tmp);
 
-  gcc_assert (calc_stmt (r, s, name));
+  calc_stmt (r, s, name);
 
   if (is_a<gphi *> (s))
     r.intersect (tmp);