[PATCH] use zero as the lower bound for a signed-unsigned range (PR 79327)

[Jakub Jelinek]

On Mon, Feb 13, 2017 at 04:53:19PM -0700, Jeff Law wrote:
> > dirtype is one of the standard {un,}signed {char,short,int,long,long long}
> > types, all of them have 0 in their ranges.
> > For VR_RANGE we almost always set res.knownrange to true:
> >           /* Set KNOWNRANGE if the argument is in a known subrange
> >              of the directive's type (KNOWNRANGE may be reset below).  */
> >           res.knownrange
> >             = (!tree_int_cst_equal (TYPE_MIN_VALUE (dirtype), argmin)
> >                || !tree_int_cst_equal (TYPE_MAX_VALUE (dirtype), argmax));
> > (the exception is in case that range clearly has to include zero),
> > and reset it only if adjust_range_for_overflow returned true, which means
> > it also set the range to TYPE_M{IN,AX}_VALUE (dirtype) and again
> > includes zero.
> > So IMNSHO likely_adjust in what you've committed is always true
> > when you use it and thus just a useless computation and something to make
> > the code harder to understand.
> If KNOWNRANGE is false, then LIKELY_ADJUST will be true.  But I don't see
> how we can determine anything for LIKELY_ADJUST if KNOWNRANGE is true.

We can't, but that doesn't matter, we only use it if KNOWNRANGE is false.
The only user of LIKELY_ADJUST is:
 
  if (res.knownrange)
    res.range.likely = res.range.max;
  else
    {
// -- Here we know res.knownrage is false
      res.range.likely = res.range.min;
      if (likely_adjust && maybebase && base != 10)
// -- and here is the only user of likely_adjust
        {
          if (res.range.min == 1)
            res.range.likely += base == 8 ? 1 : 2;
          else if (res.range.min == 2
                   && base == 16
                   && (dir.width[0] == 2 || dir.prec[0] == 2))
            ++res.range.likely;
        }
    }

> > Even if you don't trust this, with the ranges in argmin/argmax, it is
> > IMHO undesirable to set it differently at the different code paths,
> > if you want to check whether the final range includes zero and at least
> > one another value, just do
> > -      if (likely_adjust && maybebase && base != 10)
> > +      if ((tree_int_cst_sgn (argmin) < 0 || tree_int_cst_sgn (argmax) > 0)
> > 	   && maybebase && base != 10)
> > Though, it is useless both for the above reason and for the reason that you
> > actually do something only:
> I'm not convinced it's useless, but it does seem advisable to bring test
> down to where it's actually used and to bse it strictly on argmin/argmax.
> Can you test a patch which does that?

That would then be (the only difference compared to the previous patch is
the last hunk) following.  I can surely test that, I'm still convinced it
would work equally if that
(tree_int_cst_sgn (argmin) < 0 || tree_int_cst_sgn (argmax) > 0)
is just gcc_checking_assert.

2017-02-14  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/79327
	* gimple-ssa-sprintf.c (format_integer): Remove likely_adjust
	variable, its initialization and use.

--- gcc/gimple-ssa-sprintf.c.jj	2017-02-04 08:43:12.000000000 +0100
+++ gcc/gimple-ssa-sprintf.c	2017-02-04 08:45:33.173709580 +0100
@@ -1232,10 +1232,6 @@ format_integer (const directive &dir, tr
        of the format string by returning [-1, -1].  */
     return fmtresult ();
 
-  /* True if the LIKELY counter should be adjusted upward from the MIN
-     counter to account for arguments with unknown values.  */
-  bool likely_adjust = false;
-
   fmtresult res;
 
   /* Using either the range the non-constant argument is in, or its
@@ -1265,14 +1261,6 @@ format_integer (const directive &dir, tr
 
 	  res.argmin = argmin;
 	  res.argmax = argmax;
-
-	  /* Set the adjustment for an argument whose range includes
-	     zero since that doesn't include the octal or hexadecimal
-	     base prefix.  */
-	  wide_int wzero = wi::zero (wi::get_precision (min));
-	  if (wi::le_p (min, wzero, SIGNED)
-	      && !wi::neg_p (max))
-	    likely_adjust = true;
 	}
       else if (range_type == VR_ANTI_RANGE)
 	{
@@ -1307,11 +1295,6 @@ format_integer (const directive &dir, tr
 
   if (!argmin)
     {
-      /* Set the adjustment for an argument whose range includes
-	 zero since that doesn't include the octal or hexadecimal
-	 base prefix.  */
-      likely_adjust = true;
-
       if (TREE_CODE (argtype) == POINTER_TYPE)
 	{
 	  argmin = build_int_cst (pointer_sized_int_node, 0);
@@ -1371,7 +1354,8 @@ format_integer (const directive &dir, tr
   else
     {
       res.range.likely = res.range.min;
-      if (likely_adjust && maybebase && base != 10)
+      if (maybebase && base != 10
+	  && (tree_int_cst_sgn (argmin) < 0 || tree_int_cst_sgn (argmax) > 0))
 	{
 	  if (res.range.min == 1)
 	    res.range.likely += base == 8 ? 1 : 2;


	Jakub