On Fri, Feb 03, 2017 at 05:39:21PM +0100, Jakub Jelinek wrote:
> Say in the http://gcc.gnu.org/ml/gcc-patches/2017-02/msg00234.html
> patch, you would with my patch need just the tree_digits part,
> and then the very last hunk in gimple-ssa-sprintf.c (with
> likely_adjust && 
> removed).  Because you do the adjustments only if !res.knownrange
> and in that case you know argmin/argmax are actually dirtype's min/max,
> so 0 must be in the range.

You've committed the patch unnecessarily complicated, see above.
The following gives the same testsuite result.

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.

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:
          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;
where if the range doesn't include zero, you would never get
res.range.min of 1 and for base == 16 also not 2.

2017-02-04  Jakub Jelinek  <ja...@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,7 @@ format_integer (const directive &dir, tr
   else
     {
       res.range.likely = res.range.min;
-      if (likely_adjust && maybebase && base != 10)
+      if (maybebase && base != 10)
        {
          if (res.range.min == 1)
            res.range.likely += base == 8 ? 1 : 2;
--- gcc/testsuite/ChangeLog.jj  2017-02-04 08:43:14.000000000 +0100
+++ gcc/testsuite/ChangeLog     2017-02-04 08:48:14.930627979 +0100
@@ -20,8 +20,8 @@
 
        PR tree-optimization/79327
        * gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Adjust.
-       * gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-13.c: New test.
-       * gcc/testsuite/gcc.dg/tree-ssa/pr79327-2.c: Ditto.
+       * gcc.dg/tree-ssa/builtin-sprintf-warn-13.c: New test.
+       * gcc.dg/tree-ssa/pr79327-2.c: Ditto.
 
 2017-02-03  Jakub Jelinek  <ja...@redhat.com>
            Martin Sebor  <mse...@redhat.com>
@@ -306,7 +306,7 @@
 2017-01-27  Vladimir Makarov  <vmaka...@redhat.com>
 
        PR tree-optimization/71374
-       * testsuite/gcc.target/i386/pr71374.c: New.
+       * gcc.target/i386/pr71374.c: New.
 
 2017-01-27  Martin Sebor  <mse...@redhat.com>
 
@@ -484,9 +484,9 @@
        * g++.dg/ext/flexary18.C: Same.
        * g++.dg/ext/flexary19.C: Same.
        * g++.dg/ext/flexary7.C: Same.
-       * gcc/testsuite/g++.dg/cpp1z/has-unique-obj-representations1.C: Same.
-       * gcc/testsuite/g++.dg/ubsan/object-size-1.C: Same.
-       * gcc/testsuite/obj-c++.dg/property/at-property-23.mm: Same.
+       * g++.dg/cpp1z/has-unique-obj-representations1.C: Same.
+       * g++.dg/ubsan/object-size-1.C: Same.
+       * obj-c++.dg/property/at-property-23.mm: Same.
 
 2017-01-25  Jakub Jelinek  <ja...@redhat.com>
 
@@ -874,10 +874,10 @@
 
 2017-01-20  Jiong Wang  <jiong.w...@arm.com>
 
-       * testsuite/gcc.target/aarch64/return_address_sign_1.c: Enable on LP64
+       * gcc.target/aarch64/return_address_sign_1.c: Enable on LP64
        only.
-       * testsuite/gcc.target/aarch64/return_address_sign_2.c: Likewise.
-       * testsuite/gcc.target/aarch64/return_address_sign_3.c: Likewise.
+       * gcc.target/aarch64/return_address_sign_2.c: Likewise.
+       * gcc.target/aarch64/return_address_sign_3.c: Likewise.
 
 2017-01-20  Nathan Sidwell  <nat...@acm.org>
 
@@ -992,7 +992,7 @@
 
 2017-01-19  Tamar Christina  <tamar.christ...@arm.com>
 
-       * gcc/testsuite/lib/target-supports.exp
+       * lib/target-supports.exp
        (check_effective_target_vect_call_copysignf): Enable for AArch64.
 
 2017-01-19  Rainer Orth  <r...@cebitec.uni-bielefeld.de>


        Jakub

Reply via email to