VRP: convert pointers of known quantity better

Message ID 1a1dae3f-814d-e284-7aa9-9d2a59b9cbcc@redhat.com
State New
Headers show
Series
  • VRP: convert pointers of known quantity better
Related show

Commit Message

Aldy Hernandez Sept. 14, 2018, 10:31 a.m.
Apparently, my work on VRP will never finish. There's an infinity of 
things that can be tweaked ;-).

First, we shouldn't drop to null/non-null when we know what the actual 
pointer value is.  For example, [1, 3] which we get when we store magic 
numbers in a pointer (p == (char *)1).

BTW, for this bit, I would much rather change range_int_cst_p to allow 
VR_ANTI_RANGE, instead of inlining as I've done.  Is there a reason 
range_int_cst_p doesn't handle anti ranges?

Also, [&foo, &foo] is known to be non-null.  Don't drop to varying.

OK?

Comments

Jeff Law Sept. 14, 2018, 9:25 p.m. | #1
On 9/14/18 4:31 AM, Aldy Hernandez wrote:
> Apparently, my work on VRP will never finish. There's an infinity of

> things that can be tweaked ;-).

> 

> First, we shouldn't drop to null/non-null when we know what the actual

> pointer value is.  For example, [1, 3] which we get when we store magic

> numbers in a pointer (p == (char *)1).

> 

> BTW, for this bit, I would much rather change range_int_cst_p to allow

> VR_ANTI_RANGE, instead of inlining as I've done.  Is there a reason

> range_int_cst_p doesn't handle anti ranges?

> 

> Also, [&foo, &foo] is known to be non-null.  Don't drop to varying.

> 

> OK?

> 

> curr.patch

> 

> commit 7f42e101d5d26ea866b739692858289a8dff4396

> Author: Aldy Hernandez <aldyh@redhat.com>

> Date:   Fri Sep 14 00:11:34 2018 +0200

> 

>             * tree-vrp.c (extract_range_from_unary_expr): Handle pointers of

>             known quantity.

> 

> diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c

> index 622ccbc2df7..22e5ee3c729 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 evaluates to a pointer of unknown quantity,

> +	 we are only interested in determining if it evaluates to NULL

> +	 [0, 0] or non-NULL (~[0, 0]).  */

> +      if (POINTER_TYPE_P (type)

> +	  && !((vr0.type == VR_RANGE

> +		|| vr0.type == VR_ANTI_RANGE)

> +	       && TREE_CODE (vr0.min) == INTEGER_CST

> +	       && TREE_CODE (vr0.max) == INTEGER_CST))

>  	{

>  	  if (!range_includes_zero_p (&vr0))

>  	    set_value_range_to_nonnull (vr, type);

I think this part is fine.  Though I question how much time we want to
spend dealing some clowns that do things like store integers into
pointer objects :-)

> @@ -1855,6 +1860,16 @@ extract_range_from_unary_expr (value_range *vr,

>  	  return;

>  	}

>  

> +      /* If we have a non-constant range that we know is non-zero (for

> +	 example [&foo, &foo] or [&foo, +MAX]), make it known, so we

> +	 don't drop to VR_VARYING later.  */

> +      if (POINTER_TYPE_P (op0_type)

> +	  && vr0.type == VR_RANGE

> +	  && (TREE_CODE (vr0.min) != INTEGER_CST

> +	      || TREE_CODE (vr0.max) != INTEGER_CST)

> +	  && !range_includes_zero_p (&vr0))

> +	set_value_range_to_nonnull (&vr0, op0_type);

> +

I don't think this is correct in the presence of weak symbols.  I think
you can query maybe_nonzero_address on the min/max and if both return >
0, then you know the result is non-null.

Jeff
Aldy Hernandez Sept. 17, 2018, 9:16 a.m. | #2
On 09/14/2018 05:25 PM, Jeff Law wrote:
> On 9/14/18 4:31 AM, Aldy Hernandez wrote:

>> Apparently, my work on VRP will never finish. There's an infinity of

>> things that can be tweaked ;-).

>>

>> First, we shouldn't drop to null/non-null when we know what the actual

>> pointer value is.  For example, [1, 3] which we get when we store magic

>> numbers in a pointer (p == (char *)1).

>>

>> BTW, for this bit, I would much rather change range_int_cst_p to allow

>> VR_ANTI_RANGE, instead of inlining as I've done.  Is there a reason

>> range_int_cst_p doesn't handle anti ranges?

>>

>> Also, [&foo, &foo] is known to be non-null.  Don't drop to varying.

>>

>> OK?

>>

>> curr.patch

>>

>> commit 7f42e101d5d26ea866b739692858289a8dff4396

>> Author: Aldy Hernandez <aldyh@redhat.com>

>> Date:   Fri Sep 14 00:11:34 2018 +0200

>>

>>              * tree-vrp.c (extract_range_from_unary_expr): Handle pointers of

>>              known quantity.

>>

>> diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c

>> index 622ccbc2df7..22e5ee3c729 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 evaluates to a pointer of unknown quantity,

>> +	 we are only interested in determining if it evaluates to NULL

>> +	 [0, 0] or non-NULL (~[0, 0]).  */

>> +      if (POINTER_TYPE_P (type)

>> +	  && !((vr0.type == VR_RANGE

>> +		|| vr0.type == VR_ANTI_RANGE)

>> +	       && TREE_CODE (vr0.min) == INTEGER_CST

>> +	       && TREE_CODE (vr0.max) == INTEGER_CST))

>>   	{

>>   	  if (!range_includes_zero_p (&vr0))

>>   	    set_value_range_to_nonnull (vr, type);

> I think this part is fine.  Though I question how much time we want to

> spend dealing some clowns that do things like store integers into

> pointer objects :-)


Hmmm.  You're right.  I take it back.  I don't want to further 
complicate things.  I never quite liked clowns as a kid.

I have another approach to cleaning up the pointer conversion code that 
I will post separately.

> 

>> @@ -1855,6 +1860,16 @@ extract_range_from_unary_expr (value_range *vr,

>>   	  return;

>>   	}

>>   

>> +      /* If we have a non-constant range that we know is non-zero (for

>> +	 example [&foo, &foo] or [&foo, +MAX]), make it known, so we

>> +	 don't drop to VR_VARYING later.  */

>> +      if (POINTER_TYPE_P (op0_type)

>> +	  && vr0.type == VR_RANGE

>> +	  && (TREE_CODE (vr0.min) != INTEGER_CST

>> +	      || TREE_CODE (vr0.max) != INTEGER_CST)

>> +	  && !range_includes_zero_p (&vr0))

>> +	set_value_range_to_nonnull (&vr0, op0_type);

>> +

> I don't think this is correct in the presence of weak symbols.  I think

> you can query maybe_nonzero_address on the min/max and if both return >

> 0, then you know the result is non-null.


I'm taking it all back.  Perhaps I'll revisit this and test with weak 
symbols later.

Thanks, and sorry for the noise.

Aldy

Patch

commit 7f42e101d5d26ea866b739692858289a8dff4396
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Fri Sep 14 00:11:34 2018 +0200

            * tree-vrp.c (extract_range_from_unary_expr): Handle pointers of
            known quantity.

diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 622ccbc2df7..22e5ee3c729 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 evaluates to a pointer of unknown quantity,
+	 we are only interested in determining if it evaluates to NULL
+	 [0, 0] or non-NULL (~[0, 0]).  */
+      if (POINTER_TYPE_P (type)
+	  && !((vr0.type == VR_RANGE
+		|| vr0.type == VR_ANTI_RANGE)
+	       && TREE_CODE (vr0.min) == INTEGER_CST
+	       && TREE_CODE (vr0.max) == INTEGER_CST))
 	{
 	  if (!range_includes_zero_p (&vr0))
 	    set_value_range_to_nonnull (vr, type);
@@ -1855,6 +1860,16 @@  extract_range_from_unary_expr (value_range *vr,
 	  return;
 	}
 
+      /* If we have a non-constant range that we know is non-zero (for
+	 example [&foo, &foo] or [&foo, +MAX]), make it known, so we
+	 don't drop to VR_VARYING later.  */
+      if (POINTER_TYPE_P (op0_type)
+	  && vr0.type == VR_RANGE
+	  && (TREE_CODE (vr0.min) != INTEGER_CST
+	      || TREE_CODE (vr0.max) != INTEGER_CST)
+	  && !range_includes_zero_p (&vr0))
+	set_value_range_to_nonnull (&vr0, op0_type);
+
       /* 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