diff mbox series

Add stmt context in simplify_using_ranges.

Message ID fe4971e0-3819-2601-8390-3bebcf704cc0@redhat.com
State New
Headers show
Series Add stmt context in simplify_using_ranges. | expand

Commit Message

Andrew MacLeod June 29, 2021, 7:09 p.m. UTC 
We added context to a lot of simplify_using_ranges, but we didn't catch 
all the places.   This provides the originating stmt to the missing 
cases which resolve a few EVRP testcases when running in ranger-only mode.

Bootstraps on x86_64-pc-linux-gnu with no regressions.  Pushed.

Andrew

Comments

Aldy Hernandez June 30, 2021, 6:20 a.m. UTC | #1 
On 6/29/21 9:09 PM, Andrew MacLeod wrote:
> We added context to a lot of simplify_using_ranges, but we didn't catch 
> all the places.   This provides the originating stmt to the missing 
> cases which resolve a few EVRP testcases when running in ranger-only mode.
> 
> Bootstraps on x86_64-pc-linux-gnu with no regressions.  Pushed.
> 
> Andrew
> 
> 

Thanks for doing this.  I've done a half-assed job at passing context 
around; probably only when it yielded a discrepancy with evrp.

>  
>  bool
> -simplify_using_ranges::op_with_boolean_value_range_p (tree op)
> +simplify_using_ranges::op_with_boolean_value_range_p (tree op, gimple *s)
>  {
>    if (TYPE_PRECISION (TREE_TYPE (op)) == 1)
>      

I know you like single letter arguments, but I find them confusing when 
the method is more than a few lines long.  Besides, "stmt" is what is 
used throughout vr-values.c.

And speaking of passing statements around, I wonder if it'd be best to 
have m_stmt and possible m_gsi as class fields.  After all, we never 
change them, and they're used by most methods.

Aldy
Andrew MacLeod June 30, 2021, 1:25 p.m. UTC | #2 
On 6/30/21 2:20 AM, Aldy Hernandez wrote:
>
>
> On 6/29/21 9:09 PM, Andrew MacLeod wrote:
>> We added context to a lot of simplify_using_ranges, but we didn't 
>> catch all the places. This provides the originating stmt to the 
>> missing cases which resolve a few EVRP testcases when running in 
>> ranger-only mode.
>>
>> Bootstraps on x86_64-pc-linux-gnu with no regressions.  Pushed.
>>
>> Andrew
>>
>>
>
> Thanks for doing this.  I've done a half-assed job at passing context 
> around; probably only when it yielded a discrepancy with evrp.
>
>>
>>  bool
>> -simplify_using_ranges::op_with_boolean_value_range_p (tree op)
>> +simplify_using_ranges::op_with_boolean_value_range_p (tree op, 
>> gimple *s)
>>  {
>>    if (TYPE_PRECISION (TREE_TYPE (op)) == 1)
>
> I know you like single letter arguments, but I find them confusing 
> when the method is more than a few lines long.  Besides, "stmt" is 
> what is used throughout vr-values.c.
>
> And speaking of passing statements around, I wonder if it'd be best to 
> have m_stmt and possible m_gsi as class fields.  After all, we never 
> change them, and they're used by most methods.
>
> Aldy
>
I think there's a revamp of simplify down the pipe anyway.

class simplify_using_ranges
{
public:
   simplify_using_ranges (class range_query *query = NULL);
   ~simplify_using_ranges ();
   void set_range_query (class range_query *q) { query = q; }

   bool simplify (gimple_stmt_iterator *);


This is really the only external API.. the call to simplify. Long term 
Im not sure that containing all the switch update management stuff att 
he bottom of the class should be contained in this class.. That seems 
like it should be a class that is used by simplifcation...  and 
simplification itself could be stateless..    kinda following the model 
of fold_using_ranges.. the the gsi and stmt can be wrapped into a source 
class if needed...

likewise we're eventually going to want to restructure the folding stuff 
that happens..    but most of this can wait until evrp and vrp are gone, 
then we can change the model a bit easier. bigger fish to fry and they 
are already way better than they were before :-)
diff mbox series

Patch

From a7e655ae4016eaf04e261ff32fc67a14ebb0e329 Mon Sep 17 00:00:00 2001
From: Andrew MacLeod <amacleod@redhat.com>
Date: Fri, 25 Jun 2021 11:24:30 -0400
Subject: [PATCH 1/3] Add stmt context in simplify_using_ranges.

There were places simplify_using_ranges was not utilzing the stmt context.

	* vr-values.c (vr_values::vrp_stmt_computes_nonzero): Use stmt.
	(simplify_using_ranges::op_with_boolean_value_range_p): Add a
	statement for location context.
	(check_for_binary_op_overflow): Ditto.
	(simplify_using_ranges::get_vr_for_comparison): Ditto.
	(simplify_using_ranges::compare_name_with_value): Ditto.
	(simplify_using_ranges::compare_names): Ditto.
	(vrp_evaluate_conditional_warnv_with_ops_using_ranges): Ditto.
	(simplify_using_ranges::simplify_truth_ops_using_ranges): Ditto.
	(simplify_using_ranges::simplify_min_or_max_using_ranges): Ditto.
	(simplify_using_ranges::simplify_internal_call_using_ranges): Ditto.
	(simplify_using_ranges::two_valued_val_range_p): Ditto.
	(simplify_using_ranges::simplify): Ditto.
	* vr-values.h: Adjust prototypes.
---
 gcc/vr-values.c | 71 ++++++++++++++++++++++++++-----------------------
 gcc/vr-values.h | 14 +++++-----
 2 files changed, 46 insertions(+), 39 deletions(-)

diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 3ae2c68499d..190676de2c0 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -429,7 +429,7 @@  vr_values::vrp_stmt_computes_nonzero (gimple *stmt)
 		  && !TYPE_OVERFLOW_WRAPS (TREE_TYPE (expr))))
 	    {
 	      const value_range_equiv *vr
-		= get_value_range (TREE_OPERAND (base, 0));
+		= get_value_range (TREE_OPERAND (base, 0), stmt);
 	      if (!range_includes_zero_p (vr))
 		return true;
 	    }
@@ -486,7 +486,7 @@  vr_values::op_with_constant_singleton_value_range (tree op)
 /* Return true if op is in a boolean [0, 1] value-range.  */
 
 bool
-simplify_using_ranges::op_with_boolean_value_range_p (tree op)
+simplify_using_ranges::op_with_boolean_value_range_p (tree op, gimple *s)
 {
   if (TYPE_PRECISION (TREE_TYPE (op)) == 1)
     return true;
@@ -500,7 +500,7 @@  simplify_using_ranges::op_with_boolean_value_range_p (tree op)
 
   /* ?? Errr, this should probably check for [0,0] and [1,1] as well
      as [0,1].  */
-  const value_range *vr = query->get_value_range (op);
+  const value_range *vr = query->get_value_range (op, s);
   return *vr == value_range (build_zero_cst (TREE_TYPE (op)),
 			     build_one_cst (TREE_TYPE (op)));
 }
@@ -1057,18 +1057,18 @@  vr_values::extract_range_from_comparison (value_range_equiv *vr,
 static bool
 check_for_binary_op_overflow (range_query *query,
 			      enum tree_code subcode, tree type,
-			      tree op0, tree op1, bool *ovf)
+			      tree op0, tree op1, bool *ovf, gimple *s = NULL)
 {
   value_range vr0, vr1;
   if (TREE_CODE (op0) == SSA_NAME)
-    vr0 = *query->get_value_range (op0);
+    vr0 = *query->get_value_range (op0, s);
   else if (TREE_CODE (op0) == INTEGER_CST)
     vr0.set (op0);
   else
     vr0.set_varying (TREE_TYPE (op0));
 
   if (TREE_CODE (op1) == SSA_NAME)
-    vr1 = *query->get_value_range (op1);
+    vr1 = *query->get_value_range (op1, s);
   else if (TREE_CODE (op1) == INTEGER_CST)
     vr1.set (op1);
   else
@@ -1980,10 +1980,11 @@  vr_values::vrp_visit_assignment_or_call (gimple *stmt, tree *output_p,
    is varying or undefined.  Uses TEM as storage for the alternate range.  */
 
 const value_range_equiv *
-simplify_using_ranges::get_vr_for_comparison (int i, value_range_equiv *tem)
+simplify_using_ranges::get_vr_for_comparison (int i, value_range_equiv *tem,
+					      gimple *s)
 {
   /* Shallow-copy equiv bitmap.  */
-  const value_range_equiv *vr = query->get_value_range (ssa_name (i));
+  const value_range_equiv *vr = query->get_value_range (ssa_name (i), s);
 
   /* If name N_i does not have a valid range, use N_i as its own
      range.  This allows us to compare against names that may
@@ -2005,10 +2006,11 @@  simplify_using_ranges::get_vr_for_comparison (int i, value_range_equiv *tem)
 tree
 simplify_using_ranges::compare_name_with_value
 				(enum tree_code comp, tree var, tree val,
-				 bool *strict_overflow_p, bool use_equiv_p)
+				 bool *strict_overflow_p, bool use_equiv_p,
+				 gimple *s)
 {
   /* Get the set of equivalences for VAR.  */
-  bitmap e = query->get_value_range (var)->equiv ();
+  bitmap e = query->get_value_range (var, s)->equiv ();
 
   /* Start at -1.  Set it to 0 if we do a comparison without relying
      on overflow, or 1 if all comparisons rely on overflow.  */
@@ -2017,7 +2019,7 @@  simplify_using_ranges::compare_name_with_value
   /* Compare vars' value range with val.  */
   value_range_equiv tem_vr;
   const value_range_equiv *equiv_vr
-    = get_vr_for_comparison (SSA_NAME_VERSION (var), &tem_vr);
+    = get_vr_for_comparison (SSA_NAME_VERSION (var), &tem_vr, s);
   bool sop = false;
   tree retval = compare_range_with_value (comp, equiv_vr, val, &sop);
   if (retval)
@@ -2044,7 +2046,7 @@  simplify_using_ranges::compare_name_with_value
 	  && prop_simulate_again_p (SSA_NAME_DEF_STMT (name)))
 	continue;
 
-      equiv_vr = get_vr_for_comparison (i, &tem_vr);
+      equiv_vr = get_vr_for_comparison (i, &tem_vr, s);
       sop = false;
       tree t = compare_range_with_value (comp, equiv_vr, val, &sop);
       if (t)
@@ -2084,12 +2086,12 @@  simplify_using_ranges::compare_name_with_value
 
 tree
 simplify_using_ranges::compare_names (enum tree_code comp, tree n1, tree n2,
-				      bool *strict_overflow_p)
+				      bool *strict_overflow_p, gimple *s)
 {
   /* Compare the ranges of every name equivalent to N1 against the
      ranges of every name equivalent to N2.  */
-  bitmap e1 = query->get_value_range (n1)->equiv ();
-  bitmap e2 = query->get_value_range (n2)->equiv ();
+  bitmap e1 = query->get_value_range (n1, s)->equiv ();
+  bitmap e2 = query->get_value_range (n2, s)->equiv ();
 
   /* Use the fake bitmaps if e1 or e2 are not available.  */
   static bitmap s_e1 = NULL, s_e2 = NULL;
@@ -2139,7 +2141,7 @@  simplify_using_ranges::compare_names (enum tree_code comp, tree n1, tree n2,
 	continue;
 
       value_range_equiv tem_vr1;
-      const value_range_equiv *vr1 = get_vr_for_comparison (i1, &tem_vr1);
+      const value_range_equiv *vr1 = get_vr_for_comparison (i1, &tem_vr1, s);
 
       tree t = NULL_TREE, retval = NULL_TREE;
       bitmap_iterator bi2;
@@ -2152,7 +2154,8 @@  simplify_using_ranges::compare_names (enum tree_code comp, tree n1, tree n2,
 	  bool sop = false;
 
 	  value_range_equiv tem_vr2;
-	  const value_range_equiv *vr2 = get_vr_for_comparison (i2, &tem_vr2);
+	  const value_range_equiv *vr2 = get_vr_for_comparison (i2, &tem_vr2,
+								s);
 
 	  t = compare_ranges (comp, vr1, vr2, &sop);
 	  if (t)
@@ -2198,11 +2201,12 @@  simplify_using_ranges::compare_names (enum tree_code comp, tree n1, tree n2,
 
 tree
 simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops_using_ranges
-    (enum tree_code code, tree op0, tree op1, bool * strict_overflow_p)
+    (enum tree_code code, tree op0, tree op1, bool * strict_overflow_p,
+     gimple *s)
 {
   const value_range_equiv *vr0, *vr1;
-  vr0 = (TREE_CODE (op0) == SSA_NAME) ? query->get_value_range (op0) : NULL;
-  vr1 = (TREE_CODE (op1) == SSA_NAME) ? query->get_value_range (op1) : NULL;
+  vr0 = (TREE_CODE (op0) == SSA_NAME) ? query->get_value_range (op0, s) : NULL;
+  vr1 = (TREE_CODE (op1) == SSA_NAME) ? query->get_value_range (op1, s) : NULL;
 
   tree res = NULL_TREE;
   if (vr0 && vr1)
@@ -2302,20 +2306,20 @@  simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops
     }
 
   if ((ret = vrp_evaluate_conditional_warnv_with_ops_using_ranges
-	       (code, op0, op1, strict_overflow_p)))
+	       (code, op0, op1, strict_overflow_p, stmt)))
     return ret;
   if (only_ranges)
     *only_ranges = false;
   /* Do not use compare_names during propagation, it's quadratic.  */
   if (TREE_CODE (op0) == SSA_NAME && TREE_CODE (op1) == SSA_NAME
       && use_equiv_p)
-    return compare_names (code, op0, op1, strict_overflow_p);
+    return compare_names (code, op0, op1, strict_overflow_p, stmt);
   else if (TREE_CODE (op0) == SSA_NAME)
     return compare_name_with_value (code, op0, op1,
-				    strict_overflow_p, use_equiv_p);
+				    strict_overflow_p, use_equiv_p, stmt);
   else if (TREE_CODE (op1) == SSA_NAME)
     return compare_name_with_value (swap_tree_comparison (code), op1, op0,
-				    strict_overflow_p, use_equiv_p);
+				    strict_overflow_p, use_equiv_p, stmt);
   return NULL_TREE;
 }
 
@@ -2929,11 +2933,11 @@  simplify_using_ranges::simplify_truth_ops_using_ranges
   gcc_assert (rhs_code == EQ_EXPR || rhs_code == NE_EXPR);
 
   op0 = gimple_assign_rhs1 (stmt);
-  if (!op_with_boolean_value_range_p (op0))
+  if (!op_with_boolean_value_range_p (op0, stmt))
     return false;
 
   op1 = gimple_assign_rhs2 (stmt);
-  if (!op_with_boolean_value_range_p (op1))
+  if (!op_with_boolean_value_range_p (op1, stmt))
     return false;
 
   /* Reduce number of cases to handle to NE_EXPR.  As there is no
@@ -3131,12 +3135,12 @@  simplify_using_ranges::simplify_min_or_max_using_ranges
   tree val;
 
   val = (vrp_evaluate_conditional_warnv_with_ops_using_ranges
-	 (LE_EXPR, op0, op1, &sop));
+	 (LE_EXPR, op0, op1, &sop, stmt));
   if (!val)
     {
       sop = false;
       val = (vrp_evaluate_conditional_warnv_with_ops_using_ranges
-	     (LT_EXPR, op0, op1, &sop));
+	     (LT_EXPR, op0, op1, &sop, stmt));
     }
 
   if (val)
@@ -4000,7 +4004,7 @@  simplify_using_ranges::simplify_internal_call_using_ranges
     return false;
   else
     type = TREE_TYPE (TREE_TYPE (gimple_call_lhs (stmt)));
-  if (!check_for_binary_op_overflow (query, subcode, type, op0, op1, &ovf)
+  if (!check_for_binary_op_overflow (query, subcode, type, op0, op1, &ovf, stmt)
       || (is_ubsan && ovf))
     return false;
 
@@ -4057,9 +4061,10 @@  simplify_using_ranges::simplify_internal_call_using_ranges
    two-values when it is true.  Return false otherwise.  */
 
 bool
-simplify_using_ranges::two_valued_val_range_p (tree var, tree *a, tree *b)
+simplify_using_ranges::two_valued_val_range_p (tree var, tree *a, tree *b,
+					       gimple *s)
 {
-  value_range vr = *query->get_value_range (var);
+  value_range vr = *query->get_value_range (var, s);
   vr.normalize_symbolics ();
   if (vr.varying_p () || vr.undefined_p ())
     return false;
@@ -4133,7 +4138,7 @@  simplify_using_ranges::simplify (gimple_stmt_iterator *gsi)
 	  tree cmp_var = NULL_TREE;
 
 	  if (TREE_CODE (rhs2) == SSA_NAME
-	      && two_valued_val_range_p (rhs2, &val1, &val2))
+	      && two_valued_val_range_p (rhs2, &val1, &val2, stmt))
 	    {
 	      /* Optimize RHS1 OP [VAL1, VAL2].  */
 	      new_rhs1 = int_const_binop (rhs_code, rhs1, val1);
@@ -4141,7 +4146,7 @@  simplify_using_ranges::simplify (gimple_stmt_iterator *gsi)
 	      cmp_var = rhs2;
 	    }
 	  else if (TREE_CODE (rhs1) == SSA_NAME
-		   && two_valued_val_range_p (rhs1, &val1, &val2))
+		   && two_valued_val_range_p (rhs1, &val1, &val2, stmt))
 	    {
 	      /* Optimize [VAL1, VAL2] OP RHS2.  */
 	      new_rhs1 = int_const_binop (rhs_code, val1, rhs2);
diff --git a/gcc/vr-values.h b/gcc/vr-values.h
index 81b9131f7f1..7fdefef2fdf 100644
--- a/gcc/vr-values.h
+++ b/gcc/vr-values.h
@@ -56,14 +56,16 @@  private:
 					       gimple *);
   bool simplify_internal_call_using_ranges (gimple_stmt_iterator *, gimple *);
 
-  bool two_valued_val_range_p (tree, tree *, tree *);
-  bool op_with_boolean_value_range_p (tree);
-  tree compare_name_with_value (enum tree_code, tree, tree, bool *, bool);
-  tree compare_names (enum tree_code, tree, tree, bool *);
-  const value_range_equiv *get_vr_for_comparison (int, value_range_equiv *);
+  bool two_valued_val_range_p (tree, tree *, tree *, gimple *);
+  bool op_with_boolean_value_range_p (tree, gimple *);
+  tree compare_name_with_value (enum tree_code, tree, tree, bool *, bool,
+				gimple *);
+  tree compare_names (enum tree_code, tree, tree, bool *, gimple *s);
+  const value_range_equiv *get_vr_for_comparison (int, value_range_equiv *,
+						  gimple *s);
   tree vrp_evaluate_conditional_warnv_with_ops_using_ranges (enum tree_code,
 							     tree, tree,
-							     bool *);
+							     bool *, gimple *s);
   void cleanup_edges_and_switches (void);
 
   /* Vectors of edges that need removing and switch statements that
-- 
2.17.2