VRP: convert pointers of known quantity better

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?

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

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