public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Aldy Hernandez <aldyh@redhat.com>
To: Andrew MacLeod <amacleod@redhat.com>,
	Richard Biener <richard.guenther@gmail.com>
Cc: gcc-patches <gcc-patches@gcc.gnu.org>, Jeff Law <law@redhat.com>
Subject: Re: [range-ops] patch 01/04: types for VR_UNDEFINED and VR_VARYING
Date: Mon, 22 Jul 2019 15:38:00 -0000	[thread overview]
Message-ID: <a7f5ced3-69f1-dfaa-00a4-bebe736d289b@redhat.com> (raw)
In-Reply-To: <27d3db25-ea95-33f4-57fb-e9a4a40d7341@redhat.com>

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

On 7/16/19 2:37 PM, Andrew MacLeod wrote:
> On 7/9/19 5:56 AM, Richard Biener wrote:
>> On Tue, Jul 9, 2019 at 9:28 AM Aldy Hernandez <aldyh@redhat.com> wrote:
>>>
>>>
>>> On 7/4/19 6:33 AM, Richard Biener wrote:
>>>> On Wed, Jul 3, 2019 at 2:17 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>>>>> On 7/3/19 7:08 AM, Richard Biener wrote:
>>>>>> On Wed, Jul 3, 2019 at 11:19 AM Aldy Hernandez <aldyh@redhat.com> 
>>>>>> wrote:
>>>>> How about we keep VARYING and UNDEFINED typeless until right before we
>>>>> call into the ranger.  At which point, we have can populate min/max
>>>>> because we have the tree_code and the type handy.  So right before we
>>>>> call into the ranger do:
>>>>>
>>>>>           if (varying_p ())
>>>>>             foo->set_varying(TYPE);
>>>>>
>>>>> This would avoid the type cache, and keep the ranger happy.
>>>> you cannot do set_varying on the static const range but instead 
>>>> you'd do
>>>>
>>>>     value_range tem (*foo);
>>>>     if (varying_p ())
>>>>      tem->set_full_range (TYPE);
>>>>
>>>> which I think we already do in some places.  Thus my question _where_
>>>> you actually need this.
>>> Basically, everywhere.  By having a type for varying/undefined, we don't
>>> have to special case anything.  Sure, we could for example, special case
>>> the invert operation for undefined / varying.  And we could special case
>>> everything dealing with ranges to handle varying and undefined, but why?
>>>    We could also pass a type argument everywhere, but that's just ugly.
>>> However, I do understand your objection to the type cache.
>>>
>>> How about the attached approach?  Set the type for varying/undefined
>>> when we know it, while avoiding touching the CONST varying.  Then right
>>> before calling the ranger, pass down a new varying node with min/max for
>>> any varyings that were still typeless until that point.
>>>
>>> I have taken care of never adding a set_varying() that was not already
>>> there.  Would this keep the const happy?
>>>
>>> Technically we don't need to set varying/undef types for every instance
>>> in VRP, but we need it at least for the code that will be shared with
>>> range-ops (extract_range_from_multiplicative_op, union, intersect, etc).
>>>    I just figured if we have the information, might as well set it for
>>> consistency.
>>>
>>> If you like this approach, I can rebase the other patches that depend on
>>> this one.
>> OK, so I went ant checked what you do for class irange which has
>> a type but no kind member (but constructors with a kind).  It also
>> uses wide_int members for storage.  For a pure integer constant
>> range representation this represents somewhat odd choices;  I'd
>> have elided the m_type member completely here, it seems fully
>> redundant.  Only range operations need to be carried out in a
>> specific type (what I was suggesting above).  Even the precision
>> encoded in the wide_int members is redundant then (I'd have
>> expected widest_int here and trailing-wide-ints for optimizing
>> storage).
> 
> What irange contains internally is a bit arbitrary.  It's really an API 
> for managing a set of subranges.  We could have used trees internally 
> just as easily, then we wouldnt need a type field. Since we were doing 
> lots of operations, rather than going back and forth from trees all the 
> time, we just used the underlying wide_int directly.   we could have 
> fiddle farted around with HOST_WIDE_INT or whatever, but wide_int is 
> there, has all the operations, and it works fine. so thats what it 
> currently is on the branch.
> 
> We are treating a range object as a unique self contained object. 
> Therefore, the range has a type so we know how to print it, and can 
> confirm before any operation that the ranges being operated on are 
> appropriately matched.  We found and opened bugzillas over the past 
> couple years for places where our code caught bugs because a range was 
> created and then operated on in a way that was not compatible with 
> another range.  I think there is a still an open one against ada(?) 
> where the switch and case  are different precision.
> 
>  From my point of view, a range object is similar to a tree node. A tree 
> node has the bits to indicate what the value is, but also associates a 
> type with those bits within the same object.  This is less error prone 
> than passing around the bits and the type separately.  As ranges are 
> starting to be used in many places outside of VRP, we should do the same 
> thing with ranges.  WIth value_range it would actually be free since 
> there is already a tree for the bounds already which contains the type.
> 
> 
> 
> 
> 
>> to fold_range/op_range?  The API also seems to be oddly
>> constrained to binary ops.  Anyway, the way you build
>> the operator table requires an awful lot of global C++ ctor
>> invocations, sth we generally try to avoid.  But I'm getting
>> into too many details here.
> 
> Its "oddly constrained" because what you are looking at  is just the 
> standardized unary/binary ops code.. ie the equivalent of 
> extract_range_from_binary_expr() and extract_range_from_unary_expr(). 
> The other ops we care about have specialized requirements, like PHIs and 
> the arbitrary numbers of parameters in a call, or anything less common 
> than one or two operands.    You are just not seeing those parts.
> 
>>
>> So - to answer your question above, I'd like you to pass down
>> a type to operations.  Because that's what is fundamentally
>> required - a range doesn't have a "type" and the current
>> value_range_base doesn't fall into the trap of needing one.
>>
>> Richard.
>>
> 
> Why is having a type associated with the data a "trap"?   Perhaps the 
> old VRP lattice idea didn't need a type with the UNDEFINED and VARYING 
> lattice values, but we're moving past lattice values and into a realm 
> where we have ranges as useful things outside of VRP, and trying to 
> shoehorn lattice values does not seem appropriate anymore.
> 
> I looked at implementing range-ops without a type in the range, and we 
> end up passing 2 parameters everywhere each time we do anything with a 
> range.  This doubled the number of parameters in most routines, and when 
> we had chains of calls, we were simply passing the type along with the 
> range.  It seems archaic to be constantly passing information around 
> instead of associating it with the range itself.  Its just another place 
> for an error to creep in.. Aldy found a place where we were creating 
> varying nodes for floats IIRC.. the type checking in the range 
> operations caught it precisely because the range returned wasn't the 
> type it was assumed to be.
> 
> That said. I believe we can do away with the need for a type with an 
> 'UNDEFINED' range.  That is not too onerous, and there doesn't really 
> seem to be too many ripple effect from have a typeless undefined range. 
> I think we can contain that, and prevents us from having to add a hack 
> to value_range for that situation.
> 
> VARYING  is another situation completely.  We adopted the term 'varying' 
> for ease of compatibility with VRP, but varying is simply a range going 
> from MIN to MAX for a type.  Removing the type from that would then 
> require we always pass a type in with every range which gets back to 
> doubling the number of of parameters everywhere for no good reason.
> 
> If we standardize value_range so that MIN and MAX are set for varying, 
> then everything works very smoothly, we can make value_range and irange 
> interchangeable and facilitate getting the range ops code into trunk.
> 
> It seems like the only reason we cant do this right now is the CONST 
> varying nodes that are returned from get_value_range().
> Looking at that routine, it seems there are only 2 cases where that can 
> be returned
>   1) we ask for an ssa-name beyond the end of the local ssa-name vector
>   2) once values_propagated is set  to true and an ssa-name has no entry.
> 
> Both seem pretty straightforward to fix...
> 1)  if we ask for an ssa-Name beyond the end, simply reallocate the 
> vector to be big enough.   I doubt this will trigger a lot, and if we 
> initially allocate it with room for an extra 10% names it'll probably 
> never trigger.  Or pick whatever number seems appropriate.
> 2) if values_propagated is true, simply allocate a node for the 
> ssa-name,  set it to varying and return it. THIs accomplishes the same 
> thing.
> 
> the number of additional nodes we allocate will be pretty minimal, and 
> it will never exceed the number of ssa-names. Then we never have to 
> worry about having CONST set for a varying node either. I see places 
> where there is special processing to avoid calling set_varying  because 
> we dont know if the vbalue_range in hand is in constant memory and would 
> cause the compiler to trap. This seems like a really bad situation, and 
> it would eliminate that issue.  We can also then eliminate the places 
> where VARYING is expanded to min/max for a given type.. instead we can 
> just pick up min/max directly.    It seems much cleaner overall.
> 
> Something like the simple attached patch would resolve that issue, and 
> remove any lurking concerns/bugs with the CONST code.
> 
> Then we can associate a type with varying, canonicalize it consistently, 
> and set MIN/MAX appropriately.   This will give us full 
> interchangeability between irange and value_range, establish a 
> solid/consistent API,  and we can move on to the next thing :-)
> 
> Does this not seem reasonable?

The attached patch takes this one step further, and adds types to 
varying (not undefined), while getting rid of vr_const_varying 
altogether.  It depends on the canonicalization patch, but I can rebase 
it to come before it if desired.  Either 01 had to depend on 02 or vice 
versa.  I had to choose :).

There is a small caveat in ipa-cp, which seems to want to create 
varyings of floats, vectors, structures, and other nonsensical things. 
I tried to plug the callers everywhere, but eventually found it easier 
with the included one-liner:

-  m_vr.set_varying ();
+  /* ?? We create all sorts of VARYING ranges for floats, structures,
+     and other types for which we obviously cannot handle as ranges.
+     We should probably avoid handling non pointers and non integers
+     throughout, but it's simpler to create a sensible VARYING here
+     and let the lattice propagate.  */
+  m_vr.set_varying (integer_type_node);

As mentioned before, I'll add ChangeLog entries if we agree on the approach.

What do y'all think?

Tested on x86-64 Linux.

Aldy

[-- Attachment #2: p.patch --]
[-- Type: text/x-patch, Size: 27713 bytes --]

commit bb0f89b2dab87b6a1d1763b28e44fcb16e14f72b
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Mon Jul 22 10:04:43 2019 +0200

    Add type to VR_VARYING.

diff --git a/gcc/gimple-ssa-evrp-analyze.c b/gcc/gimple-ssa-evrp-analyze.c
index 4c68af847e1..0184703a13c 100644
--- a/gcc/gimple-ssa-evrp-analyze.c
+++ b/gcc/gimple-ssa-evrp-analyze.c
@@ -251,13 +251,18 @@ evrp_range_analyzer::record_ranges_from_phis (basic_block bb)
       if (virtual_operand_p (lhs))
 	continue;
 
+      /* Skips floats and other things we can't represent in a
+	 range.  */
+      if (!value_range_base::supports_type_p (TREE_TYPE (lhs)))
+	continue;
+
       value_range vr_result;
       bool interesting = stmt_interesting_for_vrp (phi);
       if (!has_unvisited_preds && interesting)
 	vr_values->extract_range_from_phi_node (phi, &vr_result);
       else
 	{
-	  vr_result.set_varying ();
+	  vr_result.set_varying (TREE_TYPE (lhs));
 	  /* When we have an unvisited executable predecessor we can't
 	     use PHI arg ranges which may be still UNDEFINED but have
 	     to use VARYING for them.  But we can still resort to
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 69c00a9c5a5..e0b85573bad 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -977,7 +977,12 @@ ipcp_vr_lattice::set_to_bottom ()
 {
   if (m_vr.varying_p ())
     return false;
-  m_vr.set_varying ();
+  /* ?? We create all sorts of VARYING ranges for floats, structures,
+     and other types for which we obviously cannot handle as ranges.
+     We should probably avoid handling non pointers and non integers
+     throughout, but it's simpler to create a sensible VARYING here
+     and let the lattice propagate.  */
+  m_vr.set_varying (integer_type_node);
   return true;
 }
 
diff --git a/gcc/tree-ssa-threadedge.c b/gcc/tree-ssa-threadedge.c
index 785227df690..43bca63da09 100644
--- a/gcc/tree-ssa-threadedge.c
+++ b/gcc/tree-ssa-threadedge.c
@@ -182,8 +182,10 @@ record_temporary_equivalences_from_phis (edge e,
 	    new_vr->deep_copy (vr_values->get_value_range (src));
 	  else if (TREE_CODE (src) == INTEGER_CST)
 	    new_vr->set (src);
+	  else if (value_range_base::supports_type_p (TREE_TYPE (src)))
+	    new_vr->set_varying (TREE_TYPE (src));
 	  else
-	    new_vr->set_varying ();
+	    new_vr->set_undefined ();
 
 	  /* This is a temporary range for DST, so push it.  */
 	  evrp_range_analyzer->push_value_range (dst, new_vr);
diff --git a/gcc/tree-ssanames.c b/gcc/tree-ssanames.c
index 8b80bce8945..3911db9c26e 100644
--- a/gcc/tree-ssanames.c
+++ b/gcc/tree-ssanames.c
@@ -440,14 +440,16 @@ get_range_info (const_tree name, value_range_base &vr)
   wide_int wmin, wmax;
   enum value_range_kind kind = get_range_info (name, &wmin, &wmax);
 
-  if (kind == VR_VARYING || kind == VR_UNDEFINED)
-    min = max = NULL;
+  if (kind == VR_VARYING)
+    vr.set_varying (TREE_TYPE (name));
+  else if (kind == VR_UNDEFINED)
+    vr.set_undefined ();
   else
     {
       min = wide_int_to_tree (TREE_TYPE (name), wmin);
       max = wide_int_to_tree (TREE_TYPE (name), wmax);
+      vr.set (kind, min, max);
     }
-  vr.set (kind, min, max);
   return kind;
 }
 
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index de2f39d8487..e215b032f78 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -186,9 +186,11 @@ value_range_base::check ()
 	break;
       }
     case VR_UNDEFINED:
-    case VR_VARYING:
       gcc_assert (!min () && !max ());
       break;
+    case VR_VARYING:
+      gcc_assert (m_min && m_max);
+      break;
     default:
       gcc_unreachable ();
     }
@@ -214,6 +216,10 @@ value_range::check ()
 bool
 value_range_base::equal_p (const value_range_base &other) const
 {
+  /* Ignore types for undefined.  All undefines are equal.  */
+  if (undefined_p ())
+    return m_kind == other.m_kind;
+
   return (m_kind == other.m_kind
 	  && vrp_operand_equal_p (m_min, other.m_min)
 	  && vrp_operand_equal_p (m_max, other.m_max));
@@ -269,16 +275,19 @@ value_range::set_undefined ()
 }
 
 void
-value_range_base::set_varying ()
+value_range_base::set_varying (tree type)
 {
+  gcc_assert (supports_type_p (type));
   m_kind = VR_VARYING;
-  m_min = m_max = NULL;
+  m_min = vrp_val_min (type, true);
+  m_max = vrp_val_max (type, true);
 }
 
 void
-value_range::set_varying ()
+value_range::set_varying (tree type)
 {
-  set (VR_VARYING, NULL, NULL, NULL);
+  value_range_base::set_varying (type);
+  equiv_clear ();
 }
 
 /* Return TRUE if it is possible that range contains VAL.  */
@@ -355,9 +364,7 @@ value_range_base::singleton_p (tree *result) const
 tree
 value_range_base::type () const
 {
-  /* Types are only valid for VR_RANGE and VR_ANTI_RANGE, which are
-     known to have non-zero min/max.  */
-  gcc_assert (min ());
+  gcc_assert (m_min || undefined_p ());
   return TREE_TYPE (min ());
 }
 
@@ -395,11 +402,20 @@ value_range_base::dump (FILE *file) const
       fprintf (file, "]");
     }
   else if (varying_p ())
-    fprintf (file, "VARYING");
+    {
+      print_generic_expr (file, type ());
+      fprintf (file, " VARYING");
+    }
   else
     gcc_unreachable ();
 }
 
+void
+value_range_base::dump () const
+{
+  dump (stderr);
+}
+
 void
 value_range::dump (FILE *file) const
 {
@@ -423,6 +439,12 @@ value_range::dump (FILE *file) const
     }
 }
 
+void
+value_range::dump () const
+{
+  dump (stderr);
+}
+
 void
 dump_value_range (FILE *file, const value_range *vr)
 {
@@ -658,7 +680,10 @@ value_range_base::set (enum value_range_kind kind, tree min, tree max)
     }
   else if (kind == VR_VARYING)
     {
-      set_varying ();
+      gcc_assert (TREE_TYPE (min) == TREE_TYPE (max));
+      gcc_assert (vrp_val_min (TREE_TYPE (min), true));
+      gcc_assert (vrp_val_max (TREE_TYPE (max), true));
+      set_varying (TREE_TYPE (min));
       return;
     }
 
@@ -683,7 +708,7 @@ value_range_base::set (enum value_range_kind kind, tree min, tree max)
 	 for VR_ANTI_RANGE empty range, so drop to varying as well.  */
       if (TYPE_PRECISION (TREE_TYPE (min)) == 1)
 	{
-	  set_varying ();
+	  set_varying (TREE_TYPE (min));
 	  return;
 	}
 
@@ -697,7 +722,7 @@ value_range_base::set (enum value_range_kind kind, tree min, tree max)
 	 to varying in this case.  */
       if (tree_int_cst_lt (max, min))
 	{
-	  set_varying ();
+	  set_varying (TREE_TYPE (min));
 	  return;
 	}
 
@@ -720,7 +745,7 @@ value_range_base::set (enum value_range_kind kind, tree min, tree max)
 	{
 	  /* We cannot deal with empty ranges, drop to varying.
 	     ???  This could be VR_UNDEFINED instead.  */
-	  set_varying ();
+	  set_varying (type);
 	  return;
 	}
       else if (TYPE_PRECISION (TREE_TYPE (min)) == 1
@@ -767,7 +792,7 @@ value_range_base::set (enum value_range_kind kind, tree min, tree max)
       && wi::eq_p (wi::to_wide (max), wi::max_value (prec, sign)))
     {
       if (kind == VR_RANGE)
-	set_varying ();
+	set_varying (type);
       else if (kind == VR_ANTI_RANGE)
 	set_undefined ();
       else
@@ -1292,7 +1317,7 @@ extract_range_from_multiplicative_op (value_range_base *vr,
 	      || code == LSHIFT_EXPR);
   if (!range_int_cst_p (vr1))
     {
-      vr->set_varying ();
+      vr->set_varying (type);
       return;
     }
 
@@ -1327,7 +1352,7 @@ extract_range_from_multiplicative_op (value_range_base *vr,
     vr->set (VR_RANGE, wide_int_to_tree (type, res_lb),
 	     wide_int_to_tree (type, res_ub));
   else
-    vr->set_varying ();
+    vr->set_varying (type);
 }
 
 /* If BOUND will include a symbolic bound, adjust it accordingly,
@@ -1536,7 +1561,7 @@ extract_range_from_binary_expr (value_range_base *vr,
   if (!INTEGRAL_TYPE_P (expr_type)
       && !POINTER_TYPE_P (expr_type))
     {
-      vr->set_varying ();
+      vr->set_varying (expr_type);
       return;
     }
 
@@ -1560,7 +1585,7 @@ extract_range_from_binary_expr (value_range_base *vr,
       && code != BIT_IOR_EXPR
       && code != BIT_XOR_EXPR)
     {
-      vr->set_varying ();
+      vr->set_varying (expr_type);
       return;
     }
 
@@ -1575,9 +1600,9 @@ extract_range_from_binary_expr (value_range_base *vr,
      have UNDEFINED result for all or some value-ranges of the not UNDEFINED
      operand.  */
   else if (vr0.undefined_p ())
-    vr0.set_varying ();
+    vr0.set_varying (expr_type);
   else if (vr1.undefined_p ())
-    vr1.set_varying ();
+    vr1.set_varying (expr_type);
 
   /* We get imprecise results from ranges_from_anti_range when
      code is EXACT_DIV_EXPR.  We could mask out bits in the resulting
@@ -1649,7 +1674,7 @@ extract_range_from_binary_expr (value_range_base *vr,
 	  || vr0.symbolic_p ()
 	  || vr1.symbolic_p ()))
     {
-      vr->set_varying ();
+      vr->set_varying (expr_type);
       return;
     }
 
@@ -1667,7 +1692,7 @@ extract_range_from_binary_expr (value_range_base *vr,
 	  else if (vr0.zero_p () && vr1.zero_p ())
 	    vr->set_zero (expr_type);
 	  else
-	    vr->set_varying ();
+	    vr->set_varying (expr_type);
 	}
       else if (code == POINTER_PLUS_EXPR)
 	{
@@ -1696,7 +1721,7 @@ extract_range_from_binary_expr (value_range_base *vr,
 	  else if (vr0.zero_p () && vr1.zero_p ())
 	    vr->set_zero (expr_type);
 	  else
-	    vr->set_varying ();
+	    vr->set_varying (expr_type);
 	}
       else if (code == BIT_AND_EXPR)
 	{
@@ -1707,10 +1732,10 @@ extract_range_from_binary_expr (value_range_base *vr,
 	  else if (vr0.zero_p () || vr1.zero_p ())
 	    vr->set_zero (expr_type);
 	  else
-	    vr->set_varying ();
+	    vr->set_varying (expr_type);
 	}
       else
-	vr->set_varying ();
+	vr->set_varying (expr_type);
 
       return;
     }
@@ -1788,7 +1813,7 @@ extract_range_from_binary_expr (value_range_base *vr,
 	  if (((bool)min_ovf && sym_min_op0 != sym_min_op1)
 	      || ((bool)max_ovf && sym_max_op0 != sym_max_op1))
 	    {
-	      vr->set_varying ();
+	      vr->set_varying (expr_type);
 	      return;
 	    }
 
@@ -1799,7 +1824,7 @@ extract_range_from_binary_expr (value_range_base *vr,
 					 wmin, wmax, min_ovf, max_ovf);
 	  if (type == VR_VARYING)
 	    {
-	      vr->set_varying ();
+	      vr->set_varying (expr_type);
 	      return;
 	    }
 
@@ -1825,7 +1850,7 @@ extract_range_from_binary_expr (value_range_base *vr,
 	     a single range or anti-range as the above is
 		 [-INF+1, +INF(OVF)] intersected with ~[5, 5]
 	     but one could use a scheme similar to equivalences for this. */
-	  vr->set_varying ();
+	  vr->set_varying (expr_type);
 	  return;
 	}
     }
@@ -1842,7 +1867,7 @@ extract_range_from_binary_expr (value_range_base *vr,
 	vr->set (VR_RANGE, wide_int_to_tree (expr_type, wmin),
 		 wide_int_to_tree (expr_type, wmax));
       else
-	vr->set_varying ();
+	vr->set_varying (expr_type);
       return;
     }
   else if (code == MULT_EXPR)
@@ -1850,7 +1875,7 @@ extract_range_from_binary_expr (value_range_base *vr,
       if (!range_int_cst_p (&vr0)
 	  || !range_int_cst_p (&vr1))
 	{
-	  vr->set_varying ();
+	  vr->set_varying (expr_type);
 	  return;
 	}
       extract_range_from_multiplicative_op (vr, code, &vr0, &vr1, expr_type);
@@ -1890,7 +1915,7 @@ extract_range_from_binary_expr (value_range_base *vr,
 		}
 	    }
 	}
-      vr->set_varying ();
+      vr->set_varying (expr_type);
       return;
     }
   else if (code == TRUNC_DIV_EXPR
@@ -1927,7 +1952,7 @@ extract_range_from_binary_expr (value_range_base *vr,
 			       TYPE_OVERFLOW_UNDEFINED (expr_type),
 			       extra_range_p, extra_min, extra_max))
 	{
-	  vr->set_varying ();
+	  vr->set_varying (expr_type);
 	  return;
 	}
       vr->set (VR_RANGE, wide_int_to_tree (expr_type, wmin),
@@ -1986,7 +2011,7 @@ extract_range_from_binary_expr (value_range_base *vr,
 	      vr->set (VR_RANGE, min, max);
 	    }
 	  else
-	    vr->set_varying ();
+	    vr->set_varying (expr_type);
 	  return;
 	}
       else if (code == BIT_IOR_EXPR)
@@ -2004,7 +2029,7 @@ extract_range_from_binary_expr (value_range_base *vr,
 	      vr->set (VR_RANGE, min, max);
 	    }
 	  else
-	    vr->set_varying ();
+	    vr->set_varying (expr_type);
 	  return;
 	}
       else if (code == BIT_XOR_EXPR)
@@ -2020,7 +2045,7 @@ extract_range_from_binary_expr (value_range_base *vr,
 	      vr->set (VR_RANGE, min, max);
 	    }
 	  else
-	    vr->set_varying ();
+	    vr->set_varying (expr_type);
 	  return;
 	}
     }
@@ -2034,7 +2059,7 @@ extract_range_from_binary_expr (value_range_base *vr,
       || max == NULL_TREE
       || TREE_OVERFLOW_P (max))
     {
-      vr->set_varying ();
+      vr->set_varying (expr_type);
       return;
     }
 
@@ -2043,7 +2068,7 @@ extract_range_from_binary_expr (value_range_base *vr,
      Note that we do accept [-INF, -INF] and [+INF, +INF].  */
   if (vrp_val_is_min (min) && vrp_val_is_max (max))
     {
-      vr->set_varying ();
+      vr->set_varying (expr_type);
       return;
     }
 
@@ -2053,7 +2078,7 @@ extract_range_from_binary_expr (value_range_base *vr,
       /* If the new range has its limits swapped around (MIN > MAX),
 	 then the operation caused one of them to wrap around, mark
 	 the new range VARYING.  */
-      vr->set_varying ();
+      vr->set_varying (expr_type);
     }
   else
     vr->set (type, min, max);
@@ -2079,7 +2104,7 @@ extract_range_from_unary_expr (value_range_base *vr,
       || !(INTEGRAL_TYPE_P (type)
 	   || POINTER_TYPE_P (type)))
     {
-      vr->set_varying ();
+      vr->set_varying (type);
       return;
     }
 
@@ -2151,7 +2176,7 @@ extract_range_from_unary_expr (value_range_base *vr,
 	  else if (vr0.zero_p ())
 	    vr->set_zero (type);
 	  else
-	    vr->set_varying ();
+	    vr->set_varying (type);
 	  return;
 	}
 
@@ -2185,7 +2210,7 @@ extract_range_from_unary_expr (value_range_base *vr,
 	  vr->set (VR_RANGE, min, max);
 	}
       else
-	vr->set_varying ();
+	vr->set_varying (outer_type);
       return;
     }
   else if (code == ABS_EXPR)
@@ -2198,7 +2223,7 @@ extract_range_from_unary_expr (value_range_base *vr,
 	vr->set (VR_RANGE, wide_int_to_tree (type, wmin),
 		 wide_int_to_tree (type, wmax));
       else
-	vr->set_varying ();
+	vr->set_varying (type);
       return;
     }
   else if (code == ABSU_EXPR)
@@ -2214,7 +2239,7 @@ extract_range_from_unary_expr (value_range_base *vr,
     }
 
   /* For unhandled operations fall back to varying.  */
-  vr->set_varying ();
+  vr->set_varying (type);
   return;
 }
 
@@ -5210,7 +5235,8 @@ vrp_prop::vrp_initialize ()
 	  if (!stmt_interesting_for_vrp (phi))
 	    {
 	      tree lhs = PHI_RESULT (phi);
-	      get_value_range (lhs)->set_varying ();
+	      if (value_range_base::supports_type_p (TREE_TYPE (lhs)))
+		get_value_range (lhs)->set_varying (TREE_TYPE (lhs));
 	      prop_set_simulate_again (phi, false);
 	    }
 	  else
@@ -5405,7 +5431,8 @@ vrp_prop::visit_stmt (gimple *stmt, edge *taken_edge_p, tree *output_p)
 	    use_operand_p use_p;
 	    enum ssa_prop_result res = SSA_PROP_VARYING;
 
-	    get_value_range (lhs)->set_varying ();
+	    if (value_range_base::supports_type_p (TREE_TYPE (lhs)))
+	      get_value_range (lhs)->set_varying (TREE_TYPE (lhs));
 
 	    FOR_EACH_IMM_USE_FAST (use_p, iter, lhs)
 	      {
@@ -6090,7 +6117,12 @@ value_range_base::intersect_helper (const value_range_base *vr0,
      VR_RANGE can still be a VR_RANGE.  Work on a temporary so we can
      fall back to vr0 when this turns things to varying.  */
   value_range_base tem;
-  tem.set (vr0type, vr0min, vr0max);
+  if (vr0type == VR_UNDEFINED)
+    tem.set_undefined ();
+  else if (vr0type == VR_VARYING)
+    tem.set_varying (vr0->type ());
+  else
+    tem.set (vr0type, vr0min, vr0max);
   /* If that failed, use the saved original VR0.  */
   if (tem.varying_p ())
     return *vr0;
@@ -6195,7 +6227,12 @@ value_range_base::union_helper (const value_range_base *vr0,
 
   /* Work on a temporary so we can still use vr0 when union returns varying.  */
   value_range_base tem;
-  tem.set (vr0type, vr0min, vr0max);
+  if (vr0type == VR_UNDEFINED)
+    tem.set_undefined ();
+  else if (vr0type == VR_VARYING)
+    tem.set_varying (vr0->type ());
+  else
+    tem.set (vr0type, vr0min, vr0max);
 
   /* Failed to find an efficient meet.  Before giving up and setting
      the result to VARYING, see if we can at least derive a useful
@@ -6292,7 +6329,7 @@ value_range_base::normalize_symbolics () const
   if (min_symbolic && max_symbolic)
     {
       value_range_base var;
-      var.set_varying ();
+      var.set_varying (ttype);
       return var;
     }
   if (kind () == VR_RANGE)
@@ -6313,7 +6350,7 @@ value_range_base::normalize_symbolics () const
 	  return value_range_base (VR_RANGE, n, vrp_val_max (ttype));
 	}
       value_range_base var;
-      var.set_varying ();
+      var.set_varying (ttype);
       return var;
     }
   // ~[NUM, SYM] -> [-MIN, NUM - 1]
@@ -6323,7 +6360,7 @@ value_range_base::normalize_symbolics () const
       return value_range_base (VR_RANGE, vrp_val_min (ttype), n);
     }
   value_range_base var;
-  var.set_varying ();
+  var.set_varying (ttype);
   return var;
 }
 
@@ -7000,7 +7037,7 @@ determine_value_range_1 (value_range_base *vr, tree expr)
 	vr->set (kind, wide_int_to_tree (TREE_TYPE (expr), min),
 		 wide_int_to_tree (TREE_TYPE (expr), max));
       else
-	vr->set_varying ();
+	vr->set_varying (TREE_TYPE (expr));
     }
 }
 
diff --git a/gcc/tree-vrp.h b/gcc/tree-vrp.h
index b5830851fcb..c879a8c6df8 100644
--- a/gcc/tree-vrp.h
+++ b/gcc/tree-vrp.h
@@ -58,7 +58,7 @@ public:
   bool constant_p () const;
   bool undefined_p () const;
   bool varying_p () const;
-  void set_varying ();
+  void set_varying (tree type);
   void set_undefined ();
 
   void union_ (const value_range_base *);
@@ -75,7 +75,9 @@ public:
   bool nonzero_p () const;
   bool singleton_p (tree *result = NULL) const;
   void dump (FILE *) const;
+  void dump () const;
 
+  static bool supports_type_p (tree);
   value_range_base normalize_symbolics () const;
 
 protected:
@@ -135,7 +137,7 @@ class GTY((user)) value_range : public value_range_base
 
   /* Types of value ranges.  */
   void set_undefined ();
-  void set_varying ();
+  void set_varying (tree);
 
   /* Equivalence bitmap methods.  */
   bitmap equiv () const;
@@ -145,6 +147,7 @@ class GTY((user)) value_range : public value_range_base
   /* Misc methods.  */
   void deep_copy (const value_range *);
   void dump (FILE *) const;
+  void dump () const;
 
  private:
   /* Deep-copies bitmap argument.  */
@@ -254,6 +257,17 @@ struct assert_info
   tree expr;
 };
 
+// Return true if TYPE is a valid type for value_range to operate on.
+// Otherwise return FALSE.
+
+inline bool
+value_range_base::supports_type_p (tree type)
+{
+  if (type && (INTEGRAL_TYPE_P (type) || POINTER_TYPE_P (type)))
+    return type;
+  return NULL;
+}
+
 extern void register_edge_assert_for (tree, edge, enum tree_code,
 				      tree, tree, vec<assert_info> &);
 extern bool stmt_interesting_for_vrp (gimple *);
diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 10789bdd64e..e2829ae985f 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -64,7 +64,7 @@ static inline void
 set_value_range_to_truthvalue (value_range *vr, tree type)
 {
   if (TYPE_PRECISION (type) == 1)
-    vr->set_varying ();
+    vr->set_varying (type);
   else
     vr->update (VR_RANGE, build_int_cst (type, 0), build_int_cst (type, 1));
 }
@@ -78,7 +78,6 @@ set_value_range_to_truthvalue (value_range *vr, tree type)
 value_range *
 vr_values::get_value_range (const_tree var)
 {
-  static const value_range vr_const_varying (VR_VARYING, NULL, NULL);
   value_range *vr;
   tree sym;
   unsigned ver = SSA_NAME_VERSION (var);
@@ -91,18 +90,35 @@ vr_values::get_value_range (const_tree var)
      We should get here at most from the substitute-and-fold stage which
      will never try to change values.  */
   if (ver >= num_vr_values)
-    return CONST_CAST (value_range *, &vr_const_varying);
+    {
+      unsigned int old_sz = num_vr_values;
+      num_vr_values = num_ssa_names + num_ssa_names / 10;
+      vr_value = XRESIZEVEC (value_range *, vr_value, num_vr_values);
+      for ( ; old_sz < num_vr_values; old_sz++)
+        vr_value [old_sz] = NULL;
+    }
 
   vr = vr_value[ver];
   if (vr)
     return vr;
 
-  /* After propagation finished do not allocate new value-ranges.  */
-  if (values_propagated)
-    return CONST_CAST (value_range *, &vr_const_varying);
 
   /* Create a default value range.  */
   vr_value[ver] = vr = vrp_value_range_pool.allocate ();
+
+  if (!value_range_base::supports_type_p (TREE_TYPE (var)))
+    {
+      vr->set_undefined ();
+      return vr;
+    }
+
+  /* After propagation finished return varying.  */
+  if (values_propagated)
+    {
+      vr->set_varying (TREE_TYPE (var));
+      return vr;
+    }
+
   vr->set_undefined ();
 
   /* If VAR is a default definition of a parameter, the variable can
@@ -126,10 +142,10 @@ vr_values::get_value_range (const_tree var)
 	    {
 	      get_range_info (var, *vr);
 	      if (vr->undefined_p ())
-		vr->set_varying ();
+		vr->set_varying (TREE_TYPE (sym));
 	    }
 	  else
-	    vr->set_varying ();
+	    vr->set_varying (TREE_TYPE (sym));
 	}
       else if (TREE_CODE (sym) == RESULT_DECL
 	       && DECL_BY_REFERENCE (sym))
@@ -150,12 +166,11 @@ vr_values::set_defs_to_varying (gimple *stmt)
   ssa_op_iter i;
   tree def;
   FOR_EACH_SSA_TREE_OPERAND (def, stmt, i, SSA_OP_DEF)
-    {
-      value_range *vr = get_value_range (def);
-      /* Avoid writing to vr_const_varying get_value_range may return.  */
-      if (!vr->varying_p ())
-	vr->set_varying ();
-    }
+    if (value_range_base::supports_type_p (TREE_TYPE (def)))
+      {
+	value_range *vr = get_value_range (def);
+	vr->set_varying (TREE_TYPE (def));
+      }
 }
 
 /* Update the value range and equivalence set for variable VAR to
@@ -198,13 +213,13 @@ vr_values::update_value_range (const_tree var, value_range *new_vr)
 	 called, if we are anyway, keep it VARYING.  */
       if (old_vr->varying_p ())
 	{
-	  new_vr->set_varying ();
+	  new_vr->set_varying (new_vr->type ());
 	  is_new = false;
 	}
       else if (new_vr->undefined_p ())
 	{
-	  old_vr->set_varying ();
-	  new_vr->set_varying ();
+	  old_vr->set_varying (TREE_TYPE (var));
+	  new_vr->set_varying (TREE_TYPE (var));
 	  return true;
 	}
       else
@@ -435,7 +450,7 @@ vr_values::extract_range_for_var_from_comparison_expr (tree var,
   if ((POINTER_TYPE_P (type) && cond_code != NE_EXPR && cond_code != EQ_EXPR)
       || limit == var)
     {
-      vr_p->set_varying ();
+      vr_p->set_varying (type);
       return;
     }
 
@@ -595,7 +610,7 @@ vr_values::extract_range_for_var_from_comparison_expr (tree var,
 	 all should be optimized away above us.  */
       if (cond_code == LT_EXPR
 	  && compare_values (max, min) == 0)
-	vr_p->set_varying ();
+	vr_p->set_varying (TREE_TYPE (min));
       else
 	{
 	  /* For LT_EXPR, we create the range [MIN, MAX - 1].  */
@@ -635,7 +650,7 @@ vr_values::extract_range_for_var_from_comparison_expr (tree var,
 	 all should be optimized away above us.  */
       if (cond_code == GT_EXPR
 	  && compare_values (min, max) == 0)
-	vr_p->set_varying ();
+	vr_p->set_varying (TREE_TYPE (min));
       else
 	{
 	  /* For GT_EXPR, we create the range [MIN + 1, MAX].  */
@@ -743,14 +758,14 @@ vr_values::extract_range_from_binary_expr (value_range *vr,
   else if (is_gimple_min_invariant (op0))
     vr0.set (op0);
   else
-    vr0.set_varying ();
+    vr0.set_varying (TREE_TYPE (op0));
 
   if (TREE_CODE (op1) == SSA_NAME)
     vr1 = *(get_value_range (op1));
   else if (is_gimple_min_invariant (op1))
     vr1.set (op1);
   else
-    vr1.set_varying ();
+    vr1.set_varying (TREE_TYPE (op1));
 
   /* If one argument is varying, we can sometimes still deduce a
      range for the output: any + [3, +INF] is in [MIN+3, +INF].  */
@@ -891,7 +906,7 @@ vr_values::extract_range_from_unary_expr (value_range *vr, enum tree_code code,
   else if (is_gimple_min_invariant (op0))
     vr0.set (op0);
   else
-    vr0.set_varying ();
+    vr0.set_varying (type);
 
   ::extract_range_from_unary_expr (vr, code, type, &vr0, TREE_TYPE (op0));
 }
@@ -913,7 +928,7 @@ vr_values::extract_range_from_cond_expr (value_range *vr, gassign *stmt)
   else if (is_gimple_min_invariant (op0))
     tem0.set (op0);
   else
-    tem0.set_varying ();
+    tem0.set_varying (TREE_TYPE (op0));
 
   tree op1 = gimple_assign_rhs3 (stmt);
   value_range tem1;
@@ -923,7 +938,7 @@ vr_values::extract_range_from_cond_expr (value_range *vr, gassign *stmt)
   else if (is_gimple_min_invariant (op1))
     tem1.set (op1);
   else
-    tem1.set_varying ();
+    tem1.set_varying (TREE_TYPE (op1));
 
   /* The resulting value range is the union of the operand ranges */
   vr->deep_copy (vr0);
@@ -975,14 +990,14 @@ vr_values::check_for_binary_op_overflow (enum tree_code subcode, tree type,
   else if (TREE_CODE (op0) == INTEGER_CST)
     vr0.set (op0);
   else
-    vr0.set_varying ();
+    vr0.set_varying (TREE_TYPE (op0));
 
   if (TREE_CODE (op1) == SSA_NAME)
     vr1 = *get_value_range (op1);
   else if (TREE_CODE (op1) == INTEGER_CST)
     vr1.set (op1);
   else
-    vr1.set_varying ();
+    vr1.set_varying (TREE_TYPE (op1));
 
   tree vr0min = vr0.min (), vr0max = vr0.max ();
   tree vr1min = vr1.min (), vr1max = vr1.max ();
@@ -1310,7 +1325,7 @@ vr_values::extract_range_basic (value_range *vr, gimple *stmt)
 	  if (vr->kind () == VR_RANGE
 	      && (vr->min () == vr->max ()
 		  || operand_equal_p (vr->min (), vr->max (), 0)))
-	    vr->set_varying ();
+	    vr->set_varying (vr->type ());
 	  return;
 	}
     }
@@ -1366,7 +1381,7 @@ vr_values::extract_range_basic (value_range *vr, gimple *stmt)
 			vr->set (build_int_cst (type, ovf));
 		      else if (TYPE_PRECISION (type) == 1
 			       && !TYPE_UNSIGNED (type))
-			vr->set_varying ();
+			vr->set_varying (type);
 		      else
 			vr->set (VR_RANGE, build_int_cst (type, 0),
 				 build_int_cst (type, 1));
@@ -1411,7 +1426,7 @@ vr_values::extract_range_basic (value_range *vr, gimple *stmt)
       vr->equiv_clear ();
     }
   else
-    vr->set_varying ();
+    vr->set_varying (type);
 }
 
 
@@ -1447,7 +1462,7 @@ vr_values::extract_range_from_assignment (value_range *vr, gassign *stmt)
 	   && is_gimple_min_invariant (gimple_assign_rhs1 (stmt)))
     vr->set (gimple_assign_rhs1 (stmt));
   else
-    vr->set_varying ();
+    vr->set_varying (TREE_TYPE (gimple_assign_lhs (stmt)));
 
   if (vr->varying_p ())
     extract_range_basic (vr, stmt);
@@ -1920,7 +1935,7 @@ vr_values::dump_all_value_ranges (FILE *file)
 vr_values::vr_values () : vrp_value_range_pool ("Tree VRP value ranges")
 {
   values_propagated = false;
-  num_vr_values = num_ssa_names;
+  num_vr_values = num_ssa_names + num_ssa_names / 10;
   vr_value = XCNEWVEC (value_range *, num_vr_values);
   vr_phi_edge_counts = XCNEWVEC (int, num_ssa_names);
   bitmap_obstack_initialize (&vrp_equiv_obstack);
@@ -2856,7 +2871,7 @@ vr_values::extract_range_from_phi_node (gphi *phi, value_range *vr_result)
 		      vr_arg_tem.set (vr_arg_->kind (), vr_arg_->min (),
 				      vr_arg_->max (), NULL);
 		      if (vr_arg_tem.symbolic_p ())
-			vr_arg_tem.set_varying ();
+			vr_arg_tem.set_varying (TREE_TYPE (arg));
 		    }
 		  else
 		    vr_arg = vr_arg_;
@@ -2978,7 +2993,7 @@ vr_values::extract_range_from_phi_node (gphi *phi, value_range *vr_result)
   goto update_range;
 
 varying:
-  vr_result->set_varying ();
+  vr_result->set_varying (TREE_TYPE (lhs));
 
 scev_check:
   /* If this is a loop PHI node SCEV may known more about its value-range.
@@ -2999,7 +3014,7 @@ infinite_check:
 	   || compare_values (vr_result->min (), vr_result->max ()) > 0))
     ;
   else
-    vr_result->set_varying ();
+    vr_result->set_varying (TREE_TYPE (lhs));
 
   /* If the new range is different than the previous value, keep
      iterating.  */

  reply	other threads:[~2019-07-22 15:37 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-01  8:52 Aldy Hernandez
2019-07-02 20:17 ` Jeff Law
2019-07-03  9:46   ` Aldy Hernandez
2019-07-03 22:40     ` Jeff Law
2019-07-03  8:28 ` Richard Biener
2019-07-03  9:19   ` Aldy Hernandez
2019-07-03 11:08     ` Richard Biener
2019-07-03 12:23       ` Aldy Hernandez
2019-07-03 12:30         ` Aldy Hernandez
2019-07-04 10:34         ` Richard Biener
2019-07-09  7:49           ` Aldy Hernandez
2019-07-09  9:57             ` Richard Biener
2019-07-16 19:56               ` Andrew MacLeod
2019-07-22 15:38                 ` Aldy Hernandez [this message]
2019-07-23  0:19                 ` Jeff Law
2019-07-23  9:45                   ` Richard Biener
2019-07-24 16:09                     ` Jeff Law
2019-07-24 17:06                       ` Richard Biener
2019-07-24 18:33                         ` Jeff Law
2019-07-24 18:38                           ` Richard Biener
2019-07-26  3:41                             ` Jeff Law
2019-07-26 14:52                               ` Andrew MacLeod
2019-07-30  9:02                                 ` Richard Biener
2019-07-30 15:16                                   ` Andrew MacLeod
2019-07-31  8:37                                     ` Richard Biener
     [not found]                                       ` <78846d0a-354e-b73a-6e15-123752038fb2@redhat.com>
2019-08-01 14:11                                         ` Richard Biener
2019-08-01 16:35                                           ` Andrew MacLeod
2019-07-26  4:34                             ` Jeff Law
2019-07-24 20:52                           ` Andrew MacLeod
2019-07-25  9:13                             ` Richard Biener
2019-07-23  9:34                 ` Richard Biener
2019-07-23 23:05                   ` Andrew MacLeod
2019-07-24 13:28                     ` Richard Biener
2019-07-24 18:07                       ` Andrew MacLeod

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a7f5ced3-69f1-dfaa-00a4-bebe736d289b@redhat.com \
    --to=aldyh@redhat.com \
    --cc=amacleod@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=law@redhat.com \
    --cc=richard.guenther@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).