Adjust gimple-ssa-sprintf.c for irange API.

This is a rather obvious patch, but I'd like a nod before committing.

Martin, I've removed your anti-range check, as it is subsumed by the
lower_bound/upper_bound code.  However, you will have to adapt the code
for multi-ranges if desired.  For example, you may want to loop through the
sub-ranges and do the right thing.  Look at value-range.h and see the comments
for class irange.  Those are the methods you should stick to.

i.e.
	for (i=0; i < vr->num_pairs(); ++i)
		stuff_with(vr->lower_bound(i), vr->upper_bound(i))

There should be no functional changes with this patch.

Aldy

gcc/ChangeLog:

	* gimple-ssa-sprintf.c (get_int_range): Adjust for irange API.
	(format_integer): Same.
	(handle_printf_call): Same.
---
 gcc/gimple-ssa-sprintf.c | 37 ++++++++++++++++---------------------
 1 file changed, 16 insertions(+), 21 deletions(-)

On 8/4/20 5:21 AM, Aldy Hernandez via Gcc-patches wrote:
> This is a rather obvious patch, but I'd like a nod before committing.
> 
> Martin, I've removed your anti-range check, as it is subsumed by the
> lower_bound/upper_bound code.  However, you will have to adapt the code
> for multi-ranges if desired.  For example, you may want to loop through the
> sub-ranges and do the right thing.  Look at value-range.h and see the comments
> for class irange.  Those are the methods you should stick to.
> 
> i.e.
> 	for (i=0; i < vr->num_pairs(); ++i)
> 		stuff_with(vr->lower_bound(i), vr->upper_bound(i))
> 
> There should be no functional changes with this patch.

I have no concern with this change but I appreciate the heads
up and the tip on how to add the multi-range support.  Just
one suggestion: I'd prefer to keep the comment about the POSIX
requirement somewhere just as a reminder.

Thanks
Martin

> 
> Aldy
> 
> gcc/ChangeLog:
> 
> 	* gimple-ssa-sprintf.c (get_int_range): Adjust for irange API.
> 	(format_integer): Same.
> 	(handle_printf_call): Same.
> ---
>   gcc/gimple-ssa-sprintf.c | 37 ++++++++++++++++---------------------
>   1 file changed, 16 insertions(+), 21 deletions(-)
> 
> diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
> index 3d77459d811..70b031fe7b9 100644
> --- a/gcc/gimple-ssa-sprintf.c
> +++ b/gcc/gimple-ssa-sprintf.c
> @@ -1070,7 +1070,7 @@ get_int_range (tree arg, HOST_WIDE_INT *pmin, HOST_WIDE_INT *pmax,
>   	  const value_range_equiv *vr
>   	    = CONST_CAST (class vr_values *, vr_values)->get_value_range (arg);
>   
> -	  if (range_int_cst_p (vr))
> +	  if (!vr->undefined_p () && !vr->varying_p () && !vr->symbolic_p ())
>   	    {
>   	      HOST_WIDE_INT type_min
>   		= (TYPE_UNSIGNED (argtype)
> @@ -1079,8 +1079,11 @@ get_int_range (tree arg, HOST_WIDE_INT *pmin, HOST_WIDE_INT *pmax,
>   
>   	      HOST_WIDE_INT type_max = tree_to_uhwi (TYPE_MAX_VALUE (argtype));
>   
> -	      *pmin = TREE_INT_CST_LOW (vr->min ());
> -	      *pmax = TREE_INT_CST_LOW (vr->max ());
> +	      tree type = TREE_TYPE (arg);
> +	      tree tmin = wide_int_to_tree (type, vr->lower_bound ());
> +	      tree tmax = wide_int_to_tree (type, vr->upper_bound ());
> +	      *pmin = TREE_INT_CST_LOW (tmin);
> +	      *pmax = TREE_INT_CST_LOW (tmax);
>   
>   	      if (*pmin < *pmax)
>   		{
> @@ -1372,10 +1375,10 @@ format_integer (const directive &dir, tree arg, const vr_values *vr_values)
>         const value_range_equiv *vr
>   	= CONST_CAST (class vr_values *, vr_values)->get_value_range (arg);
>   
> -      if (range_int_cst_p (vr))
> +      if (!vr->varying_p () && !vr->undefined_p () && !vr->symbolic_p ())
>   	{
> -	  argmin = vr->min ();
> -	  argmax = vr->max ();
> +	  argmin = wide_int_to_tree (TREE_TYPE (arg), vr->lower_bound ());
> +	  argmax = wide_int_to_tree (TREE_TYPE (arg), vr->upper_bound ());
>   
>   	  /* Set KNOWNRANGE if the argument is in a known subrange
>   	     of the directive's type and neither width nor precision
> @@ -1388,11 +1391,7 @@ format_integer (const directive &dir, tree arg, const vr_values *vr_values)
>   	  res.argmin = argmin;
>   	  res.argmax = argmax;
>   	}
> -      else if (vr->kind () == VR_ANTI_RANGE)
> -	{
> -	  /* Handle anti-ranges if/when bug 71690 is resolved.  */
> -	}
> -      else if (vr->varying_p () || vr->undefined_p ())
> +      else
>   	{
>   	  /* The argument here may be the result of promoting the actual
>   	     argument to int.  Try to determine the type of the actual
> @@ -4561,10 +4560,13 @@ handle_printf_call (gimple_stmt_iterator *gsi, const vr_values *vr_values)
>   	  const value_range_equiv *vr
>   	    = CONST_CAST (class vr_values *, vr_values)->get_value_range (size);
>   
> -	  if (range_int_cst_p (vr))
> +	  if (!vr->undefined_p () && !vr->symbolic_p ())
>   	    {
> -	      unsigned HOST_WIDE_INT minsize = TREE_INT_CST_LOW (vr->min ());
> -	      unsigned HOST_WIDE_INT maxsize = TREE_INT_CST_LOW (vr->max ());
> +	      tree type = TREE_TYPE (size);
> +	      tree tmin = wide_int_to_tree (type, vr->lower_bound ());
> +	      tree tmax = wide_int_to_tree (type, vr->upper_bound ());
> +	      unsigned HOST_WIDE_INT minsize = TREE_INT_CST_LOW (tmin);
> +	      unsigned HOST_WIDE_INT maxsize = TREE_INT_CST_LOW (tmax);
>   	      dstsize = warn_level < 2 ? maxsize : minsize;
>   
>   	      if (minsize > target_int_max ())
> @@ -4578,13 +4580,6 @@ handle_printf_call (gimple_stmt_iterator *gsi, const vr_values *vr_values)
>   	      if (maxsize > target_int_max ())
>   		posunder4k = false;
>   	    }
> -	  else if (vr->varying_p ())
> -	    {
> -	      /* POSIX requires snprintf to fail if DSTSIZE is greater
> -		 than INT_MAX.  Since SIZE's range is unknown, avoid
> -		 folding.  */
> -	      posunder4k = false;
> -	    }
>   
>   	  /* The destination size is not constant.  If the function is
>   	     bounded (e.g., snprintf) a lower bound of zero doesn't
>

On Tue, Aug 4, 2020 at 3:59 PM Martin Sebor <msebor@gmail.com> wrote:
>
> On 8/4/20 5:21 AM, Aldy Hernandez via Gcc-patches wrote:
> > This is a rather obvious patch, but I'd like a nod before committing.
> >
> > Martin, I've removed your anti-range check, as it is subsumed by the
> > lower_bound/upper_bound code.  However, you will have to adapt the code
> > for multi-ranges if desired.  For example, you may want to loop through the
> > sub-ranges and do the right thing.  Look at value-range.h and see the comments
> > for class irange.  Those are the methods you should stick to.
> >
> > i.e.
> >       for (i=0; i < vr->num_pairs(); ++i)
> >               stuff_with(vr->lower_bound(i), vr->upper_bound(i))
> >
> > There should be no functional changes with this patch.
>
> I have no concern with this change but I appreciate the heads
> up and the tip on how to add the multi-range support.  Just
> one suggestion: I'd prefer to keep the comment about the POSIX
> requirement somewhere just as a reminder.

The comment is still there, as you had a duplicate one further up:

 else if (dstsize > target_int_max ())
        {
          warning_at (gimple_location (info.callstmt), info.warnopt (),
              "specified bound %wu exceeds %<INT_MAX%>",
              dstsize);
          /* POSIX requires snprintf to fail if DSTSIZE is greater
         than INT_MAX.  Avoid folding in that case.  */
          posunder4k = false;
        }

Are you ok with this, or would you rather me copy that comment somewhere else?

Thanks.
Aldy

On 8/4/20 8:11 AM, Aldy Hernandez wrote:
> On Tue, Aug 4, 2020 at 3:59 PM Martin Sebor <msebor@gmail.com> wrote:
>>
>> On 8/4/20 5:21 AM, Aldy Hernandez via Gcc-patches wrote:
>>> This is a rather obvious patch, but I'd like a nod before committing.
>>>
>>> Martin, I've removed your anti-range check, as it is subsumed by the
>>> lower_bound/upper_bound code.  However, you will have to adapt the code
>>> for multi-ranges if desired.  For example, you may want to loop through the
>>> sub-ranges and do the right thing.  Look at value-range.h and see the comments
>>> for class irange.  Those are the methods you should stick to.
>>>
>>> i.e.
>>>        for (i=0; i < vr->num_pairs(); ++i)
>>>                stuff_with(vr->lower_bound(i), vr->upper_bound(i))
>>>
>>> There should be no functional changes with this patch.
>>
>> I have no concern with this change but I appreciate the heads
>> up and the tip on how to add the multi-range support.  Just
>> one suggestion: I'd prefer to keep the comment about the POSIX
>> requirement somewhere just as a reminder.
> 
> The comment is still there, as you had a duplicate one further up:
> 
>   else if (dstsize > target_int_max ())
>          {
>            warning_at (gimple_location (info.callstmt), info.warnopt (),
>                "specified bound %wu exceeds %<INT_MAX%>",
>                dstsize);
>            /* POSIX requires snprintf to fail if DSTSIZE is greater
>           than INT_MAX.  Avoid folding in that case.  */
>            posunder4k = false;
>          }
> 
> Are you ok with this, or would you rather me copy that comment somewhere else?

I'm fine with it as is, I didn't see the other copy.

Thanks!
Martin