On Sat, Feb 04, 2017 at 09:07:23AM +0100, Jakub Jelinek wrote: > 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.
Now bootstrapped/regtested on x86_64-linux and i686-linux (ignore the testsuite/ChangeLog part, I've committed that part in another commit already), ok for trunk? > --- 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; Jakub