diff mbox series

VRP: special case all pointer conversion code

Message ID 68461388-8d72-c1e4-85e7-2398b01c528f@redhat.com
State New
Headers show
Series VRP: special case all pointer conversion code | expand

Commit Message

Aldy Hernandez Sept. 17, 2018, 10:12 a.m. UTC 
It seems most of the remaining anti range code in 
extract_range_from_unary_expr for CONVERT_EXPR_P is actually dealing 
with non-nullness in practice.

Anti-range handling is mostly handled by canonicalizing anti-ranges into 
its two set constituents (~[10,20] => [MIN,9] U [21,MAX]) and dealing 
with them piece-meal.  For that matter, the only way we can reach the 
conversion code in extract_range_from_unary_expr with an anti-range is 
either with a pointer (because pointers are ignored from 
ranges_from_anti_range on purpose), or when converting integers of the 
form ~[SSA, SSA].  I verified this with a bootstrap + tests with some 
specially placed asserts, BTW.

So... if we special handle pointer conversions (both to and fro, as 
opposed to just to), we get rid of any anti-ranges with the exception of 
~[SSA, SSA] between integers.  And anti-ranges of unknown quantities 
(SSAs) will be handled automatically already (courtesy of 
extract_range_into_wide_ints).

I propose we handle pointers at the beginning, and everything else just 
falls into place, with no special code.

As commented in the code, this will pessimize conversions from (char 
*)~[0, 2] to int, because we will forget that the range can also not be 
1 or 2.  But as Jeff commented, we really only care about null or 
non-nullness.  Special handling magic pointers with constants IMO is a 
wasted effort.  For that matter, I think it was me that added this 
spaghetti a few weeks ago to make sure we handled ~[0,2].  We weren't 
even handling it a short while back :-).  Furthermore, in a bootstrap, I 
think we only triggered this twice.  And I'm not even sure we make 
further use of anything null/not-null for pointers later on.

This patch simplifies the code, and removes more special handling and 
cryptic comments related to anti-ranges.

Tested with all languages including Ada and Go.

OK for trunk?

Comments

Aldy Hernandez Sept. 26, 2018, 5:12 p.m. UTC | #1 
PING

On 9/17/18 6:12 AM, Aldy Hernandez wrote:
> It seems most of the remaining anti range code in 
> extract_range_from_unary_expr for CONVERT_EXPR_P is actually dealing 
> with non-nullness in practice.
> 
> Anti-range handling is mostly handled by canonicalizing anti-ranges into 
> its two set constituents (~[10,20] => [MIN,9] U [21,MAX]) and dealing 
> with them piece-meal.  For that matter, the only way we can reach the 
> conversion code in extract_range_from_unary_expr with an anti-range is 
> either with a pointer (because pointers are ignored from 
> ranges_from_anti_range on purpose), or when converting integers of the 
> form ~[SSA, SSA].  I verified this with a bootstrap + tests with some 
> specially placed asserts, BTW.
> 
> So... if we special handle pointer conversions (both to and fro, as 
> opposed to just to), we get rid of any anti-ranges with the exception of 
> ~[SSA, SSA] between integers.  And anti-ranges of unknown quantities 
> (SSAs) will be handled automatically already (courtesy of 
> extract_range_into_wide_ints).
> 
> I propose we handle pointers at the beginning, and everything else just 
> falls into place, with no special code.
> 
> As commented in the code, this will pessimize conversions from (char 
> *)~[0, 2] to int, because we will forget that the range can also not be 
> 1 or 2.  But as Jeff commented, we really only care about null or 
> non-nullness.  Special handling magic pointers with constants IMO is a 
> wasted effort.  For that matter, I think it was me that added this 
> spaghetti a few weeks ago to make sure we handled ~[0,2].  We weren't 
> even handling it a short while back :-).  Furthermore, in a bootstrap, I 
> think we only triggered this twice.  And I'm not even sure we make 
> further use of anything null/not-null for pointers later on.
> 
> This patch simplifies the code, and removes more special handling and 
> cryptic comments related to anti-ranges.
> 
> Tested with all languages including Ada and Go.
> 
> OK for trunk?
>
Aldy Hernandez Oct. 2, 2018, 3:18 p.m. UTC | #2 
PING * 2


-------- Forwarded Message --------
Subject: PING: Re: VRP: special case all pointer conversion code
Date: Wed, 26 Sep 2018 13:12:19 -0400
From: Aldy Hernandez <aldyh@redhat.com>
To: gcc-patches <gcc-patches@gcc.gnu.org>
CC: Jeff Law <law@redhat.com>

PING

On 9/17/18 6:12 AM, Aldy Hernandez wrote:
> It seems most of the remaining anti range code in 
> extract_range_from_unary_expr for CONVERT_EXPR_P is actually dealing 
> with non-nullness in practice.
> 
> Anti-range handling is mostly handled by canonicalizing anti-ranges into 
> its two set constituents (~[10,20] => [MIN,9] U [21,MAX]) and dealing 
> with them piece-meal.  For that matter, the only way we can reach the 
> conversion code in extract_range_from_unary_expr with an anti-range is 
> either with a pointer (because pointers are ignored from 
> ranges_from_anti_range on purpose), or when converting integers of the 
> form ~[SSA, SSA].  I verified this with a bootstrap + tests with some 
> specially placed asserts, BTW.
> 
> So... if we special handle pointer conversions (both to and fro, as 
> opposed to just to), we get rid of any anti-ranges with the exception of 
> ~[SSA, SSA] between integers.  And anti-ranges of unknown quantities 
> (SSAs) will be handled automatically already (courtesy of 
> extract_range_into_wide_ints).
> 
> I propose we handle pointers at the beginning, and everything else just 
> falls into place, with no special code.
> 
> As commented in the code, this will pessimize conversions from (char 
> *)~[0, 2] to int, because we will forget that the range can also not be 
> 1 or 2.  But as Jeff commented, we really only care about null or 
> non-nullness.  Special handling magic pointers with constants IMO is a 
> wasted effort.  For that matter, I think it was me that added this 
> spaghetti a few weeks ago to make sure we handled ~[0,2].  We weren't 
> even handling it a short while back :-).  Furthermore, in a bootstrap, I 
> think we only triggered this twice.  And I'm not even sure we make 
> further use of anything null/not-null for pointers later on.
> 
> This patch simplifies the code, and removes more special handling and 
> cryptic comments related to anti-ranges.
> 
> Tested with all languages including Ada and Go.
> 
> OK for trunk?
>
Jeff Law Oct. 3, 2018, 4:34 p.m. UTC | #3 
On 9/17/18 4:12 AM, Aldy Hernandez wrote:
> It seems most of the remaining anti range code in
> extract_range_from_unary_expr for CONVERT_EXPR_P is actually dealing
> with non-nullness in practice.
> 
> Anti-range handling is mostly handled by canonicalizing anti-ranges into
> its two set constituents (~[10,20] => [MIN,9] U [21,MAX]) and dealing
> with them piece-meal.  For that matter, the only way we can reach the
> conversion code in extract_range_from_unary_expr with an anti-range is
> either with a pointer (because pointers are ignored from
> ranges_from_anti_range on purpose), or when converting integers of the
> form ~[SSA, SSA].  I verified this with a bootstrap + tests with some
> specially placed asserts, BTW.
> 
> So... if we special handle pointer conversions (both to and fro, as
> opposed to just to), we get rid of any anti-ranges with the exception of
> ~[SSA, SSA] between integers.  And anti-ranges of unknown quantities
> (SSAs) will be handled automatically already (courtesy of
> extract_range_into_wide_ints).
> 
> I propose we handle pointers at the beginning, and everything else just
> falls into place, with no special code.
> 
> As commented in the code, this will pessimize conversions from (char
> *)~[0, 2] to int, because we will forget that the range can also not be
> 1 or 2.  But as Jeff commented, we really only care about null or
> non-nullness.  Special handling magic pointers with constants IMO is a
> wasted effort.  For that matter, I think it was me that added this
> spaghetti a few weeks ago to make sure we handled ~[0,2].  We weren't
> even handling it a short while back :-).  Furthermore, in a bootstrap, I
> think we only triggered this twice.  And I'm not even sure we make
> further use of anything null/not-null for pointers later on.
> 
> This patch simplifies the code, and removes more special handling and
> cryptic comments related to anti-ranges.
> 
> Tested with all languages including Ada and Go.
> 
> OK for trunk?
> 
> 
> curr.patch
> 
> commit 10406735080ebba81f31b9e7b36247446e07fb69
> Author: Aldy Hernandez <aldyh@redhat.com>
> Date:   Mon Sep 17 11:05:57 2018 +0200
> 
>             * tree-vrp.c (extract_range_from_unary_expr): Special case all
>             pointer conversions.
>             Do not do anything special for anti-ranges.
OK.
jeff
diff mbox series

Patch

commit 10406735080ebba81f31b9e7b36247446e07fb69
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Mon Sep 17 11:05:57 2018 +0200

            * tree-vrp.c (extract_range_from_unary_expr): Special case all
            pointer conversions.
            Do not do anything special for anti-ranges.

diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 622ccbc2df7..3e12d1d9bda 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -1842,9 +1842,14 @@  extract_range_from_unary_expr (value_range *vr,
       tree inner_type = op0_type;
       tree outer_type = type;
 
-      /* If the expression evaluates to a pointer, we are only interested in
-	 determining if it evaluates to NULL [0, 0] or non-NULL (~[0, 0]).  */
-      if (POINTER_TYPE_P (type))
+      /* If the expression involves a pointer, we are only interested in
+	 determining if it evaluates to NULL [0, 0] or non-NULL (~[0, 0]).
+
+	 This may lose precision when converting (char *)~[0,2] to
+	 int, because we'll forget that the pointer can also not be 1
+	 or 2.  In practice we don't care, as this is some idiot
+	 storing a magic constant to a pointer.  */
+      if (POINTER_TYPE_P (type) || POINTER_TYPE_P (op0_type))
 	{
 	  if (!range_includes_zero_p (&vr0))
 	    set_value_range_to_nonnull (vr, type);
@@ -1855,15 +1860,12 @@  extract_range_from_unary_expr (value_range *vr,
 	  return;
 	}
 
-      /* We normalize everything to a VR_RANGE, but for constant
-	 anti-ranges we must handle them by leaving the final result
-	 as an anti range.  This allows us to convert things like
-	 ~[0,5] seamlessly.  */
-      value_range_type vr_type = VR_RANGE;
-      if (vr0.type == VR_ANTI_RANGE
-	  && TREE_CODE (vr0.min) == INTEGER_CST
-	  && TREE_CODE (vr0.max) == INTEGER_CST)
-	vr_type = VR_ANTI_RANGE;
+      /* The POINTER_TYPE_P code above will have dealt with all
+	 pointer anti-ranges.  Any remaining anti-ranges at this point
+	 will be integer conversions from SSA names that will be
+	 normalized into VARYING.  For instance: ~[x_55, x_55].  */
+      gcc_assert (vr0.type != VR_ANTI_RANGE
+		  || TREE_CODE (vr0.min) != INTEGER_CST);
 
       /* NOTES: Previously we were returning VARYING for all symbolics, but
 	 we can do better by treating them as [-MIN, +MAX].  For
@@ -1886,7 +1888,7 @@  extract_range_from_unary_expr (value_range *vr,
 	{
 	  tree min = wide_int_to_tree (outer_type, wmin);
 	  tree max = wide_int_to_tree (outer_type, wmax);
-	  set_and_canonicalize_value_range (vr, vr_type, min, max, NULL);
+	  set_and_canonicalize_value_range (vr, VR_RANGE, min, max, NULL);
 	}
       else
 	set_value_range_to_varying (vr);